From d3732796b6c6367d2059bbca4dcf70ca043bfed5 Mon Sep 17 00:00:00 2001 From: Victor Costa Date: Tue, 16 Feb 2016 16:15:41 -0300 Subject: [PATCH] Do not store serialized objects in target_data --- db/migrate/20160216134912_clean_target_data_column.rb | 27 +++++++++++++++++++++++++++ lib/merit_ext.rb | 16 +++++++++++++--- script/check_merit_actions_vs_points.rb | 4 +++- script/process_merit_rules.rb | 2 ++ test/unit/article_test.rb | 7 +++++++ test/unit/merit_ext_test.rb | 8 ++++++++ 6 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20160216134912_clean_target_data_column.rb diff --git a/db/migrate/20160216134912_clean_target_data_column.rb b/db/migrate/20160216134912_clean_target_data_column.rb new file mode 100644 index 0000000..82cd13c --- /dev/null +++ b/db/migrate/20160216134912_clean_target_data_column.rb @@ -0,0 +1,27 @@ +class CleanTargetDataColumn < ActiveRecord::Migration + + def up + models = {'article' => 'Article', 'comment' => 'Comment', 'vote' => 'Vote', + 'friendship' => 'Friendship', 'profile' => 'Profile', + 'articlefollower' => 'ArticleFollower'} + + Merit::Action.where("target_model NOT IN (?)", models.keys).find_each do |action| + next if action.target_data.blank? + obj = YAML.load(action.target_data) rescue nil + unless obj.nil? + action.update_attribute(:target_model, obj.class.respond_to?(:base_class) ? obj.class.base_class.name : obj.class.name) + end + end + + models.each do |old_name, new_name| + Merit::Action.where(target_model: old_name).update_all(target_model: new_name) + end + + Merit::Action.update_all(target_data: nil) + end + + def down + puts "Warning: cannot restore target_data" + end + +end diff --git a/lib/merit_ext.rb b/lib/merit_ext.rb index 8e9a447..e933bb7 100644 --- a/lib/merit_ext.rb +++ b/lib/merit_ext.rb @@ -1,6 +1,7 @@ require 'merit/badge_ext' require 'merit/sash' require 'merit/badges_sash' +require 'merit/action' module Merit @@ -40,6 +41,16 @@ module Merit end end + class Action + def target_obj + target_model.constantize.find_by_id(target_id) + end + + def rules_matcher + @rules_matcher ||= ::Merit::RulesMatcher.new(target_model.downcase, action_method) + end + end + module ClassMethods def has_merit_actions(options = {}) @@ -64,9 +75,8 @@ module Merit :user_id => user ? user.id : nil, :action_method => action, :had_errors => self.errors.present?, - :target_model => self.class.base_class.name.downcase, - :target_id => self.id, - :target_data => self.to_yaml + :target_model => self.class.base_class.name, + :target_id => self.id }) action.check_all_rules action diff --git a/script/check_merit_actions_vs_points.rb b/script/check_merit_actions_vs_points.rb index 010dec0..dff2097 100644 --- a/script/check_merit_actions_vs_points.rb +++ b/script/check_merit_actions_vs_points.rb @@ -17,7 +17,9 @@ class ProcessObserver merit = changed_data[:merit_object] if merit.kind_of?(Merit::Score::Point) action = Merit::Action.find(changed_data[:merit_action_id]) - new_date = YAML.load(action.target_data).created_at + model = action.target_obj + return if model.nil? + new_date = model.created_at action.update_attribute(:created_at, new_date) merit.update_attribute(:created_at, new_date) end diff --git a/script/process_merit_rules.rb b/script/process_merit_rules.rb index 76ad56c..db7e624 100755 --- a/script/process_merit_rules.rb +++ b/script/process_merit_rules.rb @@ -6,6 +6,8 @@ class ProcessObserver merit = changed_data[:merit_object] if merit.kind_of?(Merit::Score::Point) action = Merit::Action.find(changed_data[:merit_action_id]) + model = action.target_obj + return if model.nil? new_date = YAML.load(action.target_data).created_at action.update_attribute(:created_at, new_date) merit.update_attribute(:created_at, new_date) diff --git a/test/unit/article_test.rb b/test/unit/article_test.rb index cb7aea7..64512bc 100644 --- a/test/unit/article_test.rb +++ b/test/unit/article_test.rb @@ -209,4 +209,11 @@ class ArticleTest < ActiveSupport::TestCase assert_equal [], person.badges end + should 'restore article object from action' do + create_point_rule_definition('article_author') + article = create(TextArticle, :profile_id => person.id, :author => person) + assert_equal 1, person.score_points.count + assert_equal article, person.score_points.first.action.target_obj + end + end diff --git a/test/unit/merit_ext_test.rb b/test/unit/merit_ext_test.rb index 0278089..1c25913 100644 --- a/test/unit/merit_ext_test.rb +++ b/test/unit/merit_ext_test.rb @@ -24,4 +24,12 @@ class MeritExtTest < ActiveSupport::TestCase assert !point.undo_rule? end + should 'return target object associated to the merit action' do + article = fast_create(Article) + action = Merit::Action.new + action.target_model = article.class.base_class.name + action.target_id = article.id + assert_equal article, action.target_obj + end + end -- libgit2 0.21.2