From 197948c4d475eb449ffcf566a913303e951578bd Mon Sep 17 00:00:00 2001 From: Leandro Nunes dos Santos Date: Tue, 28 Sep 2010 09:35:53 -0300 Subject: [PATCH] Remove duplicate notification (ActionItem1714) --- ...20100928000952_aggressive_indexing_strategy2.rb | 17 +++ lib/notify_activity_to_profiles_job.rb | 9 +- test/unit/action_tracker_notification_test.rb | 28 ++++ test/unit/article_test.rb | 137 ++++++++++++++++++++ 4 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20100928000952_aggressive_indexing_strategy2.rb diff --git a/db/migrate/20100928000952_aggressive_indexing_strategy2.rb b/db/migrate/20100928000952_aggressive_indexing_strategy2.rb new file mode 100644 index 0000000..46fe9d9 --- /dev/null +++ b/db/migrate/20100928000952_aggressive_indexing_strategy2.rb @@ -0,0 +1,17 @@ +class AggressiveIndexingStrategy2 < ActiveRecord::Migration + def self.up + add_index(:action_tracker, [:target_type, :target_id]) + add_index(:action_tracker, [:user_type, :user_id]) + add_index(:action_tracker_notifications, :profile_id) + add_index(:action_tracker_notifications, :action_tracker_id) + add_index(:action_tracker_notifications, [:profile_id, :action_tracker_id], :unique => true) + end + + def self.down + remove_index(:action_tracker, [:target_type, :target_id]) + remove_index(:action_tracker, [:user_type, :user_id]) + remove_index(:action_tracker_notifications, :profile_id) + remove_index(:action_tracker_notifications, :action_tracker_id) + remove_index(:action_tracker_notifications, [:profile_id, :action_tracker_id]) + end +end diff --git a/lib/notify_activity_to_profiles_job.rb b/lib/notify_activity_to_profiles_job.rb index aa948e9..577fcec 100644 --- a/lib/notify_activity_to_profiles_job.rb +++ b/lib/notify_activity_to_profiles_job.rb @@ -12,17 +12,18 @@ class NotifyActivityToProfilesJob < Struct.new(:tracked_action_id) tracked_action = ActionTracker::Record.find(tracked_action_id) target = tracked_action.target if target.is_a?(Community) && NOTIFY_ONLY_COMMUNITY.include?(tracked_action.verb) - ActionTrackerNotification.connection.execute("insert into action_tracker_notifications(profile_id, action_tracker_id) values (#{target.id}, #{tracked_action.id})") + ActionTrackerNotification.create(:profile_id => target.id, :action_tracker_id => tracked_action.id) return end - ActionTrackerNotification.connection.execute("insert into action_tracker_notifications(profile_id, action_tracker_id) values (#{tracked_action.user.id}, #{tracked_action.id})") - ActionTrackerNotification.connection.execute("insert into action_tracker_notifications(profile_id, action_tracker_id) select friend_id, #{tracked_action.id} from friendships where person_id=#{tracked_action.user.id}") + ActionTrackerNotification.create(:profile_id => tracked_action.user.id, :action_tracker_id => tracked_action.id) + + ActionTrackerNotification.connection.execute("insert into action_tracker_notifications(profile_id, action_tracker_id) select f.friend_id, #{tracked_action.id} from friendships as f where person_id=#{tracked_action.user.id} and f.friend_id not in (select atn.profile_id from action_tracker_notifications as atn where atn.action_tracker_id = #{tracked_action.id})") if target.is_a?(Community) ActionTrackerNotification.connection.execute("insert into action_tracker_notifications(profile_id, action_tracker_id) select distinct profiles.id, #{tracked_action.id} from role_assignments, profiles where profiles.type = 'Person' and profiles.id = role_assignments.accessor_id and profiles.id != #{tracked_action.user.id} and role_assignments.resource_type = 'Profile' and role_assignments.resource_id = #{target.id}") - ActionTrackerNotification.connection.execute("insert into action_tracker_notifications(profile_id, action_tracker_id) values (#{target.id}, #{tracked_action.id})") unless NOT_NOTIFY_COMMUNITY.include?(tracked_action.verb) + ActionTrackerNotification.create(:profile_id => target.id, :action_tracker_id => tracked_action.id) unless NOT_NOTIFY_COMMUNITY.include?(tracked_action.verb) end end end diff --git a/test/unit/action_tracker_notification_test.rb b/test/unit/action_tracker_notification_test.rb index c0128ef..83ef102 100644 --- a/test/unit/action_tracker_notification_test.rb +++ b/test/unit/action_tracker_notification_test.rb @@ -48,4 +48,32 @@ class ActionTrackerNotificationTest < ActiveSupport::TestCase assert_equal count, ActionTrackerNotification.count end + should "the action_tracker_id be unique on scope of profile" do + atn = fast_create(ActionTrackerNotification, :action_tracker_id => 1, :profile_id => 1) + assert atn.valid? + + atn = ActionTrackerNotification.new(:action_tracker_id => 1, :profile_id => 1) + atn.valid? + assert atn.errors.invalid?(:action_tracker_id) + + atn.profile_id = 2 + atn.valid? + assert !atn.errors.invalid?(:action_tracker_id) + end + + should "the action_tracker_id be unique on scope of profile when created by ActionTracker::Record association" do + at = fast_create(ActionTracker::Record) + person = fast_create(Person) + assert_equal [], at.action_tracker_notifications + at.action_tracker_notifications<< ActionTrackerNotification.new(:profile => person) + at.reload + + assert_equal 1, at.action_tracker_notifications.count + last_notification = at.action_tracker_notifications.first + + at.action_tracker_notifications<< ActionTrackerNotification.new(:profile => person) + at.reload + assert_equal [last_notification], at.action_tracker_notifications + end + end diff --git a/test/unit/article_test.rb b/test/unit/article_test.rb index 7d6bc0d..6ed2869 100644 --- a/test/unit/article_test.rb +++ b/test/unit/article_test.rb @@ -1056,4 +1056,141 @@ class ArticleTest < Test::Unit::TestCase assert_equal false, a.is_trackable? end + should 'not create more than one notification track action to community when update more than one artile' do + community = fast_create(Community) + p1 = Person.first || fast_create(Person) + community.add_member(p1) + assert p1.is_member_of?(community) + Article.destroy_all + ActionTracker::Record.destroy_all + article = TinyMceArticle.create! :name => 'Tracked Article 1', :profile_id => community.id + assert article.published? + assert_kind_of Community, article.profile + assert_equal 1, ActionTracker::Record.count + ta = ActionTracker::Record.last + assert_equal 'Tracked Article 1', ta.get_name.last + assert_equal article.url, ta.get_url.last + assert p1, ta.user + assert community, ta.target + process_delayed_job_queue + assert_equal 2, ActionTrackerNotification.count + + article = TinyMceArticle.create! :name => 'Tracked Article 2', :profile_id => community.id + assert article.published? + assert_kind_of Community, article.profile + assert_equal 1, ActionTracker::Record.count + ta = ActionTracker::Record.last + assert_equal 'Tracked Article 2', ta.get_name.last + assert_equal article.url, ta.get_url.last + assert_equal p1, ta.user + assert_equal community, ta.target + process_delayed_job_queue + assert_equal 2, ActionTrackerNotification.count + end + + should 'create the notification to the member when one member has the notification and the other no' do + community = fast_create(Community) + p1 = Person.first || fast_create(Person) + community.add_member(p1) + assert p1.is_member_of?(community) + Article.destroy_all + ActionTracker::Record.destroy_all + article = TinyMceArticle.create! :name => 'Tracked Article 1', :profile_id => community.id + assert article.published? + assert_kind_of Community, article.profile + assert_equal 1, ActionTracker::Record.count + ta = ActionTracker::Record.first + assert_equal 'Tracked Article 1', ta.get_name.last + assert_equal article.url, ta.get_url.last + assert p1, ta.user + assert community, ta.target + process_delayed_job_queue + assert_equal 2, ActionTrackerNotification.count + + p2 = fast_create(Person) + community.add_member(p2) + process_delayed_job_queue + assert_equal 5, ActionTrackerNotification.count + + article = TinyMceArticle.create! :name => 'Tracked Article 2', :profile_id => community.id + assert article.published? + assert_kind_of Community, article.profile + assert_equal 3, ActionTracker::Record.count + ta = ActionTracker::Record.first + assert_equal 'Tracked Article 2', ta.get_name.last + assert_equal article.url, ta.get_url.last + assert_equal p1, ta.user + assert_equal community, ta.target + process_delayed_job_queue + assert_equal 6, ActionTrackerNotification.count + end + + should 'not create more than one notification track action to friends when update more than one artile' do + p1 = Person.first || fast_create(Person) + friend = fast_create(Person) + p1.add_friend(friend) + Article.destroy_all + ActionTracker::Record.destroy_all + ActionTrackerNotification.destroy_all + article = TinyMceArticle.create! :name => 'Tracked Article 1', :profile_id => p1.id + assert article.published? + assert_kind_of Person, article.profile + assert_equal 1, ActionTracker::Record.count + ta = ActionTracker::Record.last + assert_equal 'Tracked Article 1', ta.get_name.last + assert_equal article.url, ta.get_url.last + assert p1, ta.user + assert p1, ta.target + process_delayed_job_queue + assert_equal 2, ActionTrackerNotification.count + + article = TinyMceArticle.create! :name => 'Tracked Article 2', :profile_id => p1.id + assert article.published? + assert_kind_of Person, article.profile + assert_equal 1, ActionTracker::Record.count + ta = ActionTracker::Record.last + assert_equal 'Tracked Article 2', ta.get_name.last + assert_equal article.url, ta.get_url.last + assert_equal p1, ta.user + assert_equal p1, ta.target + process_delayed_job_queue + assert_equal 2, ActionTrackerNotification.count + end + + should 'create the notification to the friend when one friend has the notification and the other no' do + p1 = Person.first || fast_create(Person) + f1 = fast_create(Person) + p1.add_friend(f1) + Article.destroy_all + ActionTracker::Record.destroy_all + ActionTrackerNotification.destroy_all + article = TinyMceArticle.create! :name => 'Tracked Article 1', :profile_id => p1.id + assert article.published? + assert_kind_of Person, article.profile + assert_equal 1, ActionTracker::Record.count + ta = ActionTracker::Record.first + assert_equal 'Tracked Article 1', ta.get_name.last + assert_equal article.url, ta.get_url.last + assert p1, ta.user + assert p1, ta.target + process_delayed_job_queue + assert_equal 2, ActionTrackerNotification.count + + f2 = fast_create(Person) + p1.add_friend(f2) + process_delayed_job_queue + assert_equal 5, ActionTrackerNotification.count + article = TinyMceArticle.create! :name => 'Tracked Article 2', :profile_id => p1.id + assert article.published? + assert_kind_of Person, article.profile + assert_equal 2, ActionTracker::Record.count + ta = ActionTracker::Record.first + assert_equal 'Tracked Article 2', ta.get_name.last + assert_equal article.url, ta.get_url.last + assert_equal p1, ta.user + assert_equal p1, ta.target + process_delayed_job_queue + assert_equal 6, ActionTrackerNotification.count + end + end -- 1.7.0.4