Commit 573331b04c2003b33e7f326671111d8dd1cfe7d0
1 parent
74d4317a
Exists in
staging
and in
26 other branches
Detroying activity when article is removed
Also: - Added migration to remove action_trackers with target nil - Updated db/schema (ActionItem2400)
Showing
6 changed files
with
73 additions
and
9 deletions
Show diff stats
app/models/article.rb
| @@ -51,6 +51,11 @@ class Article < ActiveRecord::Base | @@ -51,6 +51,11 @@ class Article < ActiveRecord::Base | ||
| 51 | end | 51 | end |
| 52 | end | 52 | end |
| 53 | 53 | ||
| 54 | + after_destroy :destroy_activity | ||
| 55 | + def destroy_activity | ||
| 56 | + self.activity.destroy if self.activity | ||
| 57 | + end | ||
| 58 | + | ||
| 54 | xss_terminate :only => [ :name ], :on => 'validation', :with => 'white_list' | 59 | xss_terminate :only => [ :name ], :on => 'validation', :with => 'white_list' |
| 55 | 60 | ||
| 56 | named_scope :in_category, lambda { |category| | 61 | named_scope :in_category, lambda { |category| |
db/migrate/20120818030329_remove_action_tracker_with_target_nil.rb
0 → 100644
| @@ -0,0 +1,14 @@ | @@ -0,0 +1,14 @@ | ||
| 1 | +class RemoveActionTrackerWithTargetNil < ActiveRecord::Migration | ||
| 2 | + def self.up | ||
| 3 | + select_all("SELECT id FROM action_tracker").each do |tracker| | ||
| 4 | + activity = ActionTracker::Record.find_by_id(tracker['id']) | ||
| 5 | + if activity && activity.target.nil? | ||
| 6 | + activity.destroy | ||
| 7 | + end | ||
| 8 | + end | ||
| 9 | + end | ||
| 10 | + | ||
| 11 | + def self.down | ||
| 12 | + say "this migration can't be reverted" | ||
| 13 | + end | ||
| 14 | +end |
db/schema.rb
| @@ -9,7 +9,7 @@ | @@ -9,7 +9,7 @@ | ||
| 9 | # | 9 | # |
| 10 | # It's strongly recommended to check this file into your version control system. | 10 | # It's strongly recommended to check this file into your version control system. |
| 11 | 11 | ||
| 12 | -ActiveRecord::Schema.define(:version => 20120813163139) do | 12 | +ActiveRecord::Schema.define(:version => 20120818030329) do |
| 13 | 13 | ||
| 14 | create_table "abuse_reports", :force => true do |t| | 14 | create_table "abuse_reports", :force => true do |t| |
| 15 | t.integer "reporter_id" | 15 | t.integer "reporter_id" |
test/unit/article_test.rb
| @@ -977,9 +977,9 @@ class ArticleTest < ActiveSupport::TestCase | @@ -977,9 +977,9 @@ class ArticleTest < ActiveSupport::TestCase | ||
| 977 | assert_equivalent [p1,p2,community], ActionTrackerNotification.find_all_by_action_tracker_id(activity.id).map(&:profile) | 977 | assert_equivalent [p1,p2,community], ActionTrackerNotification.find_all_by_action_tracker_id(activity.id).map(&:profile) |
| 978 | end | 978 | end |
| 979 | 979 | ||
| 980 | - should 'not track action when a published article is removed' do | 980 | + should 'destroy activity when a published article is removed' do |
| 981 | a = create(TinyMceArticle, :profile_id => profile.id) | 981 | a = create(TinyMceArticle, :profile_id => profile.id) |
| 982 | - assert_no_difference ActionTracker::Record, :count do | 982 | + assert_difference ActionTracker::Record, :count, -1 do |
| 983 | a.destroy | 983 | a.destroy |
| 984 | end | 984 | end |
| 985 | end | 985 | end |
| @@ -1115,6 +1115,51 @@ class ArticleTest < ActiveSupport::TestCase | @@ -1115,6 +1115,51 @@ class ArticleTest < ActiveSupport::TestCase | ||
| 1115 | assert_equal 3, ActionTrackerNotification.find_all_by_action_tracker_id(article2.activity.id).count | 1115 | assert_equal 3, ActionTrackerNotification.find_all_by_action_tracker_id(article2.activity.id).count |
| 1116 | end | 1116 | end |
| 1117 | 1117 | ||
| 1118 | + should 'destroy activity and notifications of friends when destroying an article' do | ||
| 1119 | + friend = fast_create(Person) | ||
| 1120 | + profile.add_friend(friend) | ||
| 1121 | + Article.destroy_all | ||
| 1122 | + ActionTracker::Record.destroy_all | ||
| 1123 | + ActionTrackerNotification.destroy_all | ||
| 1124 | + UserStampSweeper.any_instance.expects(:current_user).returns(profile).at_least_once | ||
| 1125 | + article = create(TinyMceArticle, :profile_id => profile.id) | ||
| 1126 | + activity = article.activity | ||
| 1127 | + | ||
| 1128 | + process_delayed_job_queue | ||
| 1129 | + assert_equal 2, ActionTrackerNotification.find_all_by_action_tracker_id(activity.id).count | ||
| 1130 | + | ||
| 1131 | + assert_difference ActionTrackerNotification, :count, -2 do | ||
| 1132 | + article.destroy | ||
| 1133 | + end | ||
| 1134 | + | ||
| 1135 | + assert_raise ActiveRecord::RecordNotFound do | ||
| 1136 | + ActionTracker::Record.find(activity.id) | ||
| 1137 | + end | ||
| 1138 | + end | ||
| 1139 | + | ||
| 1140 | + should 'destroy action_tracker and notifications when an article is destroyed in a community' do | ||
| 1141 | + community = fast_create(Community) | ||
| 1142 | + p1 = fast_create(Person) | ||
| 1143 | + p2 = fast_create(Person) | ||
| 1144 | + community.add_member(p1) | ||
| 1145 | + community.add_member(p2) | ||
| 1146 | + UserStampSweeper.any_instance.expects(:current_user).returns(p1).at_least_once | ||
| 1147 | + | ||
| 1148 | + article = create(TinyMceArticle, :profile_id => community.id) | ||
| 1149 | + activity = article.activity | ||
| 1150 | + | ||
| 1151 | + process_delayed_job_queue | ||
| 1152 | + assert_equal 3, ActionTrackerNotification.find_all_by_action_tracker_id(activity.id).count | ||
| 1153 | + | ||
| 1154 | + assert_difference ActionTrackerNotification, :count, -3 do | ||
| 1155 | + article.destroy | ||
| 1156 | + end | ||
| 1157 | + | ||
| 1158 | + assert_raise ActiveRecord::RecordNotFound do | ||
| 1159 | + ActionTracker::Record.find(activity.id) | ||
| 1160 | + end | ||
| 1161 | + end | ||
| 1162 | + | ||
| 1118 | should 'found articles with published date between a range' do | 1163 | should 'found articles with published date between a range' do |
| 1119 | start_date = DateTime.parse('2010-07-06') | 1164 | start_date = DateTime.parse('2010-07-06') |
| 1120 | end_date = DateTime.parse('2010-08-02') | 1165 | end_date = DateTime.parse('2010-08-02') |
test/unit/textile_article_test.rb
| @@ -70,20 +70,20 @@ class TextileArticleTest < ActiveSupport::TestCase | @@ -70,20 +70,20 @@ class TextileArticleTest < ActiveSupport::TestCase | ||
| 70 | end | 70 | end |
| 71 | end | 71 | end |
| 72 | 72 | ||
| 73 | - should 'not notify activity on destroy' do | 73 | + should 'remove activity after destroying article' do |
| 74 | ActionTracker::Record.delete_all | 74 | ActionTracker::Record.delete_all |
| 75 | a = TextileArticle.create! :name => 'bar', :profile_id => fast_create(Profile).id, :published => true | 75 | a = TextileArticle.create! :name => 'bar', :profile_id => fast_create(Profile).id, :published => true |
| 76 | - assert_no_difference ActionTracker::Record, :count do | 76 | + assert_difference ActionTracker::Record, :count, -1 do |
| 77 | a.destroy | 77 | a.destroy |
| 78 | end | 78 | end |
| 79 | end | 79 | end |
| 80 | 80 | ||
| 81 | - should 'not notify when an article is destroyed' do | 81 | + should 'remove activity after article is destroyed' do |
| 82 | ActionTracker::Record.delete_all | 82 | ActionTracker::Record.delete_all |
| 83 | a1 = TextileArticle.create! :name => 'bar', :profile_id => fast_create(Profile).id, :published => true | 83 | a1 = TextileArticle.create! :name => 'bar', :profile_id => fast_create(Profile).id, :published => true |
| 84 | a2 = TextileArticle.create! :name => 'another bar', :profile_id => fast_create(Profile).id, :published => true | 84 | a2 = TextileArticle.create! :name => 'another bar', :profile_id => fast_create(Profile).id, :published => true |
| 85 | assert_equal 2, ActionTracker::Record.count | 85 | assert_equal 2, ActionTracker::Record.count |
| 86 | - assert_no_difference ActionTracker::Record, :count do | 86 | + assert_difference ActionTracker::Record, :count, -2 do |
| 87 | a1.destroy | 87 | a1.destroy |
| 88 | a2.destroy | 88 | a2.destroy |
| 89 | end | 89 | end |
test/unit/tiny_mce_article_test.rb
| @@ -168,11 +168,11 @@ class TinyMceArticleTest < ActiveSupport::TestCase | @@ -168,11 +168,11 @@ class TinyMceArticleTest < ActiveSupport::TestCase | ||
| 168 | end | 168 | end |
| 169 | end | 169 | end |
| 170 | 170 | ||
| 171 | - should 'not notify when an article is destroyed' do | 171 | + should 'remove activity when an article is destroyed' do |
| 172 | ActionTracker::Record.delete_all | 172 | ActionTracker::Record.delete_all |
| 173 | a1 = TinyMceArticle.create! :name => 'bar', :profile_id => fast_create(Profile).id, :published => true | 173 | a1 = TinyMceArticle.create! :name => 'bar', :profile_id => fast_create(Profile).id, :published => true |
| 174 | a2 = TinyMceArticle.create! :name => 'another bar', :profile_id => fast_create(Profile).id, :published => true | 174 | a2 = TinyMceArticle.create! :name => 'another bar', :profile_id => fast_create(Profile).id, :published => true |
| 175 | - assert_no_difference ActionTracker::Record, :count do | 175 | + assert_difference ActionTracker::Record, :count, -2 do |
| 176 | a1.destroy | 176 | a1.destroy |
| 177 | a2.destroy | 177 | a2.destroy |
| 178 | end | 178 | end |