Commit 8eb914c0811e0235b9ae926f712061785ca40c06
1 parent
5fba93dd
Exists in
master
and in
29 other branches
action_tracker: register and use activities_count to fetch most_active profile
(ActionItem3039)
Showing
10 changed files
with
158 additions
and
64 deletions
Show diff stats
app/models/organization.rb
@@ -32,13 +32,6 @@ class Organization < Profile | @@ -32,13 +32,6 @@ class Organization < Profile | ||
32 | :joins => "LEFT OUTER JOIN role_assignments ON profiles.id = role_assignments.resource_id", | 32 | :joins => "LEFT OUTER JOIN role_assignments ON profiles.id = role_assignments.resource_id", |
33 | :order => "total DESC" | 33 | :order => "total DESC" |
34 | 34 | ||
35 | - named_scope :more_active, | ||
36 | - :select => "#{Profile.qualified_column_names}, count(action_tracker.id) as total", | ||
37 | - :joins => "LEFT OUTER JOIN action_tracker ON profiles.id = action_tracker.target_id", | ||
38 | - :group => Profile.qualified_column_names, | ||
39 | - :order => 'total DESC', | ||
40 | - :conditions => ['action_tracker.created_at >= ? OR action_tracker.id IS NULL', ActionTracker::Record::RECENT_DELAY.days.ago] | ||
41 | - | ||
42 | def validation_methodology | 35 | def validation_methodology |
43 | self.validation_info ? self.validation_info.validation_methodology : nil | 36 | self.validation_info ? self.validation_info.validation_methodology : nil |
44 | end | 37 | end |
app/models/person.rb
@@ -68,13 +68,6 @@ class Person < Profile | @@ -68,13 +68,6 @@ class Person < Profile | ||
68 | 68 | ||
69 | named_scope :more_popular, :order => 'friends_count DESC' | 69 | named_scope :more_popular, :order => 'friends_count DESC' |
70 | 70 | ||
71 | - named_scope :more_active, | ||
72 | - :select => "#{Profile.qualified_column_names}, count(action_tracker.id) as total", | ||
73 | - :joins => "LEFT OUTER JOIN action_tracker ON profiles.id = action_tracker.user_id", | ||
74 | - :group => Profile.qualified_column_names, | ||
75 | - :order => 'total DESC', | ||
76 | - :conditions => ['action_tracker.created_at >= ? OR action_tracker.id IS NULL', ActionTracker::Record::RECENT_DELAY.days.ago] | ||
77 | - | ||
78 | named_scope :abusers, :joins => :abuse_complaints, :conditions => ['tasks.status = 3'], :select => 'DISTINCT profiles.*' | 71 | named_scope :abusers, :joins => :abuse_complaints, :conditions => ['tasks.status = 3'], :select => 'DISTINCT profiles.*' |
79 | named_scope :non_abusers, :joins => "LEFT JOIN tasks ON profiles.id = tasks.requestor_id AND tasks.type='AbuseComplaint'", :conditions => ["tasks.status != 3 OR tasks.id is NULL"], :select => "DISTINCT profiles.*" | 72 | named_scope :non_abusers, :joins => "LEFT JOIN tasks ON profiles.id = tasks.requestor_id AND tasks.type='AbuseComplaint'", :conditions => ["tasks.status != 3 OR tasks.id is NULL"], :select => "DISTINCT profiles.*" |
80 | 73 |
app/models/profile.rb
@@ -113,10 +113,11 @@ class Profile < ActiveRecord::Base | @@ -113,10 +113,11 @@ class Profile < ActiveRecord::Base | ||
113 | 113 | ||
114 | named_scope :visible, :conditions => { :visible => true } | 114 | named_scope :visible, :conditions => { :visible => true } |
115 | named_scope :public, :conditions => { :visible => true, :public_profile => true } | 115 | named_scope :public, :conditions => { :visible => true, :public_profile => true } |
116 | - # Subclasses must override these methods | 116 | + |
117 | + # Subclasses must override this method | ||
117 | named_scope :more_popular | 118 | named_scope :more_popular |
118 | - named_scope :more_active | ||
119 | 119 | ||
120 | + named_scope :more_active, :order => 'activities_count DESC' | ||
120 | named_scope :more_recent, :order => "created_at DESC" | 121 | named_scope :more_recent, :order => "created_at DESC" |
121 | 122 | ||
122 | acts_as_trackable :dependent => :destroy | 123 | acts_as_trackable :dependent => :destroy |
db/migrate/20140312151857_define_initial_value_for_profiles_activities_count.rb
0 → 100644
@@ -0,0 +1,14 @@ | @@ -0,0 +1,14 @@ | ||
1 | +class DefineInitialValueForProfilesActivitiesCount < ActiveRecord::Migration | ||
2 | + def self.up | ||
3 | + person_activities_counts = execute("SELECT profiles.id, count(action_tracker.id) as count FROM profiles LEFT OUTER JOIN action_tracker ON profiles.id = action_tracker.user_id WHERE (action_tracker.created_at >= '#{30.days.ago.to_s(:db)}') AND ( (profiles.type = 'Person' ) ) GROUP BY profiles.id;") | ||
4 | + organization_activities_counts = execute("SELECT profiles.id, count(action_tracker.id) as count FROM profiles LEFT OUTER JOIN action_tracker ON profiles.id = action_tracker.target_id WHERE (action_tracker.created_at >= '#{30.days.ago.to_s(:db)}') AND ( (profiles.type = 'Community' OR profiles.type = 'Enterprise' OR profiles.type = 'Organization' ) ) GROUP BY profiles.id;") | ||
5 | + activities_counts = person_activities_counts.entries + organization_activities_counts.entries | ||
6 | + activities_counts.each do |count| | ||
7 | + execute("UPDATE profiles SET activities_count=#{count['count'].to_i} WHERE profiles.id=#{count['id']};") | ||
8 | + end | ||
9 | + end | ||
10 | + | ||
11 | + def self.down | ||
12 | + execute("UPDATE profiles SET activities_count=0;") | ||
13 | + end | ||
14 | +end |
@@ -0,0 +1,11 @@ | @@ -0,0 +1,11 @@ | ||
1 | +class ActivitiesCounterCacheJob | ||
2 | + def perform | ||
3 | + person_activities_counts = ActiveRecord::Base.connection.execute("SELECT profiles.id, count(action_tracker.id) as count FROM profiles LEFT OUTER JOIN action_tracker ON profiles.id = action_tracker.user_id WHERE (action_tracker.created_at >= '#{ActionTracker::Record::RECENT_DELAY.days.ago.to_s(:db)}') AND ( (profiles.type = 'Person' ) ) GROUP BY profiles.id;") | ||
4 | + organization_activities_counts = ActiveRecord::Base.connection.execute("SELECT profiles.id, count(action_tracker.id) as count FROM profiles LEFT OUTER JOIN action_tracker ON profiles.id = action_tracker.target_id WHERE (action_tracker.created_at >= '#{ActionTracker::Record::RECENT_DELAY.days.ago.to_s(:db)}') AND ( (profiles.type = 'Community' OR profiles.type = 'Enterprise' OR profiles.type = 'Organization' ) ) GROUP BY profiles.id;") | ||
5 | + activities_counts = person_activities_counts.entries + organization_activities_counts.entries | ||
6 | + activities_counts.each do |count| | ||
7 | + ActiveRecord::Base.connection.execute("UPDATE profiles SET activities_count=#{count['count'].to_i} WHERE profiles.id=#{count['id']};") | ||
8 | + end | ||
9 | + Delayed::Job.enqueue(ActivitiesCounterCacheJob.new, -3, 1.day.from_now) | ||
10 | + end | ||
11 | +end |
@@ -0,0 +1,34 @@ | @@ -0,0 +1,34 @@ | ||
1 | +require File.dirname(__FILE__) + '/../test_helper' | ||
2 | + | ||
3 | +class ActivitiesCounterCacheJobTest < ActiveSupport::TestCase | ||
4 | + | ||
5 | + should 'correctly update the person activities counter cache' do | ||
6 | + person = create_user('person').person | ||
7 | + ActionTracker::Record.create!(:user => person, :verb => 'create_article') | ||
8 | + ActionTracker::Record.create!(:user => person, :verb => 'create_article') | ||
9 | + assert_equal 2, person.activities_count | ||
10 | + | ||
11 | + person.activities_count = 0 | ||
12 | + person.save! | ||
13 | + job = ActivitiesCounterCacheJob.new | ||
14 | + job.perform | ||
15 | + person.reload | ||
16 | + assert_equal 2, person.activities_count | ||
17 | + end | ||
18 | + | ||
19 | + should 'correctly update the organization activities counter cache' do | ||
20 | + person = create_user('person').person | ||
21 | + organization = Organization.create!(:name => 'Organization1', :identifier => 'organization1') | ||
22 | + ActionTracker::Record.create!(:user => person, :verb => 'create_article', :target => organization) | ||
23 | + ActionTracker::Record.create!(:user => person, :verb => 'create_article', :target => organization) | ||
24 | + assert_equal 2, organization.activities_count | ||
25 | + | ||
26 | + organization.activities_count = 0 | ||
27 | + organization.save! | ||
28 | + job = ActivitiesCounterCacheJob.new | ||
29 | + job.perform | ||
30 | + organization.reload | ||
31 | + assert_equal 2, organization.activities_count | ||
32 | + end | ||
33 | + | ||
34 | +end |
test/unit/organization_test.rb
@@ -364,25 +364,7 @@ class OrganizationTest < ActiveSupport::TestCase | @@ -364,25 +364,7 @@ class OrganizationTest < ActiveSupport::TestCase | ||
364 | fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => person, :created_at => Time.now, :target_id => p3.id) | 364 | fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => person, :created_at => Time.now, :target_id => p3.id) |
365 | fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => person, :created_at => Time.now, :target_id => p3.id) | 365 | fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => person, :created_at => Time.now, :target_id => p3.id) |
366 | 366 | ||
367 | - assert_equal [p3,p2,p1] , Organization.more_active | ||
368 | - end | ||
369 | - | ||
370 | - should 'more active profile take in consideration only actions created only in the recent delay interval' do | ||
371 | - ActionTracker::Record.destroy_all | ||
372 | - recent_delay = ActionTracker::Record::RECENT_DELAY.days.ago | ||
373 | - | ||
374 | - person = fast_create(Person) | ||
375 | - Organization.destroy_all | ||
376 | - p1 = fast_create(Organization) | ||
377 | - p2 = fast_create(Organization) | ||
378 | - | ||
379 | - ActionTracker::Record.destroy_all | ||
380 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => person, :created_at => recent_delay, :target_id => p1.id) | ||
381 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => person, :created_at => recent_delay, :target_id => p1.id) | ||
382 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => person, :created_at => recent_delay, :target_id => p2.id) | ||
383 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => person, :created_at => recent_delay - 1.day, :target_id => p2.id) | ||
384 | - | ||
385 | - assert_equal [p1,p2] , Organization.more_active | 367 | + assert_order [p3,p2,p1] , Organization.more_active |
386 | end | 368 | end |
387 | 369 | ||
388 | should 'list profiles that have no actions in more active list' do | 370 | should 'list profiles that have no actions in more active list' do |
@@ -422,4 +404,34 @@ class OrganizationTest < ActiveSupport::TestCase | @@ -422,4 +404,34 @@ class OrganizationTest < ActiveSupport::TestCase | ||
422 | assert !organization.visible | 404 | assert !organization.visible |
423 | end | 405 | end |
424 | 406 | ||
407 | + should 'increase activities_count on new activity' do | ||
408 | + person = fast_create(Person) | ||
409 | + organization = fast_create(Organization) | ||
410 | + assert_difference organization, :activities_count, 1 do | ||
411 | + ActionTracker::Record.create! :verb => :leave_scrap, :user => person, :target => organization | ||
412 | + organization.reload | ||
413 | + end | ||
414 | + end | ||
415 | + | ||
416 | + should 'decrease activities_count on activity removal' do | ||
417 | + person = fast_create(Person) | ||
418 | + organization = fast_create(Organization) | ||
419 | + record = ActionTracker::Record.create! :verb => :leave_scrap, :user => person, :target => organization | ||
420 | + assert_difference organization, :activities_count, -1 do | ||
421 | + record.destroy | ||
422 | + organization.reload | ||
423 | + end | ||
424 | + end | ||
425 | + | ||
426 | + should 'not decrease activities_count on activity removal after the recent delay' do | ||
427 | + person = fast_create(Person) | ||
428 | + organization = fast_create(Organization) | ||
429 | + record = ActionTracker::Record.create! :verb => :leave_scrap, :user => person, :target => organization, :created_at => (ActionTracker::Record::RECENT_DELAY + 1).days.ago | ||
430 | + assert_no_difference organization, :activities_count do | ||
431 | + record.destroy | ||
432 | + organization.reload | ||
433 | + end | ||
434 | + end | ||
435 | + | ||
436 | + | ||
425 | end | 437 | end |
test/unit/person_test.rb
@@ -904,14 +904,15 @@ class PersonTest < ActiveSupport::TestCase | @@ -904,14 +904,15 @@ class PersonTest < ActiveSupport::TestCase | ||
904 | 904 | ||
905 | action_tracker = fast_create(ActionTracker::Record, :verb => 'create_article') | 905 | action_tracker = fast_create(ActionTracker::Record, :verb => 'create_article') |
906 | action_tracker.target = community | 906 | action_tracker.target = community |
907 | + action_tracker.user = p4 | ||
907 | action_tracker.save! | 908 | action_tracker.save! |
908 | ActionTrackerNotification.delete_all | 909 | ActionTrackerNotification.delete_all |
909 | - assert_difference(ActionTrackerNotification, :count, 3) do | 910 | + assert_difference(ActionTrackerNotification, :count, 4) do |
910 | Person.notify_activity(action_tracker) | 911 | Person.notify_activity(action_tracker) |
911 | process_delayed_job_queue | 912 | process_delayed_job_queue |
912 | end | 913 | end |
913 | ActionTrackerNotification.all.map{|a|a.profile}.map do |profile| | 914 | ActionTrackerNotification.all.map{|a|a.profile}.map do |profile| |
914 | - assert [community,p1,p3].include?(profile) | 915 | + assert [community,p1,p3,p4].include?(profile) |
915 | end | 916 | end |
916 | end | 917 | end |
917 | 918 | ||
@@ -1014,9 +1015,9 @@ class PersonTest < ActiveSupport::TestCase | @@ -1014,9 +1015,9 @@ class PersonTest < ActiveSupport::TestCase | ||
1014 | 1015 | ||
1015 | should 'the community specific notification created when a member joins community could not be propagated to members' do | 1016 | should 'the community specific notification created when a member joins community could not be propagated to members' do |
1016 | ActionTracker::Record.delete_all | 1017 | ActionTracker::Record.delete_all |
1017 | - p1 = create_user('test_user').person | ||
1018 | - p2 = create_user('test_user').person | ||
1019 | - p3 = create_user('test_user').person | 1018 | + p1 = create_user('p1').person |
1019 | + p2 = create_user('p2').person | ||
1020 | + p3 = create_user('p3').person | ||
1020 | c = fast_create(Community, :name => "Foo") | 1021 | c = fast_create(Community, :name => "Foo") |
1021 | c.add_member(p1) | 1022 | c.add_member(p1) |
1022 | process_delayed_job_queue | 1023 | process_delayed_job_queue |
@@ -1136,30 +1137,14 @@ class PersonTest < ActiveSupport::TestCase | @@ -1136,30 +1137,14 @@ class PersonTest < ActiveSupport::TestCase | ||
1136 | p3 = fast_create(Person) | 1137 | p3 = fast_create(Person) |
1137 | 1138 | ||
1138 | ActionTracker::Record.destroy_all | 1139 | ActionTracker::Record.destroy_all |
1139 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p1, :created_at => Time.now) | ||
1140 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p2, :created_at => Time.now) | ||
1141 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p2, :created_at => Time.now) | ||
1142 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p3, :created_at => Time.now) | ||
1143 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p3, :created_at => Time.now) | ||
1144 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p3, :created_at => Time.now) | ||
1145 | - | ||
1146 | - assert_equal [p3,p2,p1] , Person.more_active | ||
1147 | - end | ||
1148 | - | ||
1149 | - should 'more active profile take in consideration only actions created only in the recent delay interval' do | ||
1150 | - Person.delete_all | ||
1151 | - ActionTracker::Record.destroy_all | ||
1152 | - recent_delay = ActionTracker::Record::RECENT_DELAY.days.ago | 1140 | + fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p1) |
1141 | + fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p2) | ||
1142 | + fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p2) | ||
1143 | + fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p3) | ||
1144 | + fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p3) | ||
1145 | + fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p3) | ||
1153 | 1146 | ||
1154 | - p1 = fast_create(Person) | ||
1155 | - p2 = fast_create(Person) | ||
1156 | - | ||
1157 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p1, :created_at => recent_delay) | ||
1158 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p1, :created_at => recent_delay) | ||
1159 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p2, :created_at => recent_delay) | ||
1160 | - fast_create(ActionTracker::Record, :user_type => 'Profile', :user_id => p2, :created_at => recent_delay - 1.day) | ||
1161 | - | ||
1162 | - assert_equal [p1,p2], Person.more_active | 1147 | + assert_order [p3,p2,p1] , Person.more_active |
1163 | end | 1148 | end |
1164 | 1149 | ||
1165 | should 'list profiles that have no actions in more active list' do | 1150 | should 'list profiles that have no actions in more active list' do |
@@ -1429,7 +1414,7 @@ class PersonTest < ActiveSupport::TestCase | @@ -1429,7 +1414,7 @@ class PersonTest < ActiveSupport::TestCase | ||
1429 | end | 1414 | end |
1430 | end | 1415 | end |
1431 | 1416 | ||
1432 | - should 'decrease friends_count on new friendship' do | 1417 | + should 'decrease friends_count on friendship removal' do |
1433 | person = create_user('person').person | 1418 | person = create_user('person').person |
1434 | friend = create_user('friend').person | 1419 | friend = create_user('friend').person |
1435 | person.add_friend(friend) | 1420 | person.add_friend(friend) |
@@ -1443,4 +1428,33 @@ class PersonTest < ActiveSupport::TestCase | @@ -1443,4 +1428,33 @@ class PersonTest < ActiveSupport::TestCase | ||
1443 | person.reload | 1428 | person.reload |
1444 | end | 1429 | end |
1445 | end | 1430 | end |
1431 | + | ||
1432 | + should 'increase activities_count on new activity' do | ||
1433 | + person = fast_create(Person) | ||
1434 | + assert_difference person, :activities_count, 1 do | ||
1435 | + ActionTracker::Record.create! :verb => :leave_scrap, :user => person, :target => fast_create(Profile) | ||
1436 | + person.reload | ||
1437 | + end | ||
1438 | + end | ||
1439 | + | ||
1440 | + should 'decrease activities_count on activity removal' do | ||
1441 | + person = fast_create(Person) | ||
1442 | + record = ActionTracker::Record.create! :verb => :leave_scrap, :user => person, :target => fast_create(Profile) | ||
1443 | + assert_difference person, :activities_count, -1 do | ||
1444 | + record.destroy | ||
1445 | + person.reload | ||
1446 | + end | ||
1447 | + end | ||
1448 | + | ||
1449 | + should 'not decrease activities_count on activity removal after the recent delay' do | ||
1450 | + person = fast_create(Person) | ||
1451 | + record = ActionTracker::Record.create! :verb => :leave_scrap, :user => person, :target => fast_create(Profile) | ||
1452 | + record.created_at = record.created_at - ActionTracker::Record::RECENT_DELAY.days - 1.day | ||
1453 | + record.save! | ||
1454 | + assert_no_difference person, :activities_count do | ||
1455 | + record.destroy | ||
1456 | + person.reload | ||
1457 | + end | ||
1458 | + end | ||
1459 | + | ||
1446 | end | 1460 | end |
vendor/plugins/action_tracker/lib/action_tracker_model.rb
1 | module ActionTracker | 1 | module ActionTracker |
2 | class Record < ActiveRecord::Base | 2 | class Record < ActiveRecord::Base |
3 | 3 | ||
4 | + extend CacheCounterHelper | ||
5 | + | ||
4 | set_table_name 'action_tracker' | 6 | set_table_name 'action_tracker' |
5 | 7 | ||
6 | belongs_to :user, :polymorphic => true | 8 | belongs_to :user, :polymorphic => true |
@@ -14,6 +16,22 @@ module ActionTracker | @@ -14,6 +16,22 @@ module ActionTracker | ||
14 | validates_presence_of :user | 16 | validates_presence_of :user |
15 | validate :user_existence | 17 | validate :user_existence |
16 | 18 | ||
19 | + after_create do |record| | ||
20 | + update_cache_counter(:activities_count, record.user, 1) | ||
21 | + if record.target.kind_of?(Organization) | ||
22 | + update_cache_counter(:activities_count, record.target, 1) | ||
23 | + end | ||
24 | + end | ||
25 | + | ||
26 | + after_destroy do |record| | ||
27 | + if record.created_at >= RECENT_DELAY.days.ago | ||
28 | + update_cache_counter(:activities_count, record.user, -1) | ||
29 | + if record.target.kind_of?(Organization) | ||
30 | + update_cache_counter(:activities_count, record.target, -1) | ||
31 | + end | ||
32 | + end | ||
33 | + end | ||
34 | + | ||
17 | def user_existence | 35 | def user_existence |
18 | errors.add(:user, "user doesn't exists") if user && !user.class.exists?(user) | 36 | errors.add(:user, "user doesn't exists") if user && !user.class.exists?(user) |
19 | end | 37 | end |