From f074c0c7341e3f5cca9f1f2486437b0189646818 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Thu, 25 Oct 2018 01:46:43 +0200 Subject: [PATCH] Followup for issue #1394 - Reindex elastic search not possible because of /Twitter::NullObject. --- ...ed_twitter_ticket_article_preferences7.rb} | 2 +- lib/tweet_base.rb | 61 +++++++++- test/unit/ticket_article_twitter_test.rb | 113 +++++++++++++++--- 3 files changed, 153 insertions(+), 23 deletions(-) rename db/migrate/{20180806000001_fixed_twitter_ticket_article_preferences6.rb => 20180806000001_fixed_twitter_ticket_article_preferences7.rb} (96%) diff --git a/db/migrate/20180806000001_fixed_twitter_ticket_article_preferences6.rb b/db/migrate/20180806000001_fixed_twitter_ticket_article_preferences7.rb similarity index 96% rename from db/migrate/20180806000001_fixed_twitter_ticket_article_preferences6.rb rename to db/migrate/20180806000001_fixed_twitter_ticket_article_preferences7.rb index 33fffa6ba..5f39b5796 100644 --- a/db/migrate/20180806000001_fixed_twitter_ticket_article_preferences6.rb +++ b/db/migrate/20180806000001_fixed_twitter_ticket_article_preferences7.rb @@ -1,4 +1,4 @@ -class FixedTwitterTicketArticlePreferences6 < ActiveRecord::Migration[5.0] +class FixedTwitterTicketArticlePreferences7 < ActiveRecord::Migration[5.0] def up # return if it's a new setup diff --git a/lib/tweet_base.rb b/lib/tweet_base.rb index bcd26bcdd..98369b021 100644 --- a/lib/tweet_base.rb +++ b/lib/tweet_base.rb @@ -366,10 +366,50 @@ class TweetBase false end +=begin + + replace Twitter::Place and Twitter::Geo as hash and replace Twitter::NullObject with nil + + preferences = TweetBase.preferences_cleanup( + twitter: twitter_preferences, + links: [ + { + url: 'https://twitter.com/statuses/123', + target: '_blank', + name: 'on Twitter', + }, + ], + ) + +or + + preferences = { + twitter: TweetBase.preferences_cleanup(twitter_preferences), + links: [ + { + url: 'https://twitter.com/statuses/123', + target: '_blank', + name: 'on Twitter', + }, + ], + } + +=end + def self.preferences_cleanup(preferences) # replace Twitter::NullObject with nill to prevent elasticsearch index issue - preferences.each_value do |value| + preferences.each do |key, value| + + if value.class == Twitter::Place || value.class == Twitter::Geo + preferences[key] = value.to_h + next + end + if value.class == Twitter::NullObject + preferences[key] = nil + next + end + next if !value.is_a?(Hash) value.each do |sub_key, sub_level| @@ -387,11 +427,20 @@ class TweetBase end end - if preferences[:geo].blank? - preferences[:geo] = {} - end - if preferences[:place].blank? - preferences[:place] = {} + if preferences[:twitter] + if preferences[:twitter][:geo].blank? + preferences[:twitter][:geo] = {} + end + if preferences[:twitter][:place].blank? + preferences[:twitter][:place] = {} + end + else + if preferences[:geo].blank? + preferences[:geo] = {} + end + if preferences[:place].blank? + preferences[:place] = {} + end end preferences diff --git a/test/unit/ticket_article_twitter_test.rb b/test/unit/ticket_article_twitter_test.rb index f36fa8ce8..d23756021 100644 --- a/test/unit/ticket_article_twitter_test.rb +++ b/test/unit/ticket_article_twitter_test.rb @@ -40,7 +40,7 @@ class TicketArticleTwitter < ActiveSupport::TestCase truncated: false } preferences = { - twitter: twitter_preferences, + twitter: TweetBase.preferences_cleanup(twitter_preferences), links: [ { url: 'https://twitter.com/statuses/123', @@ -56,18 +56,58 @@ class TicketArticleTwitter < ActiveSupport::TestCase from: '@example', body: 'some tweet', internal: false, - preferences: TweetBase.preferences_cleanup(preferences), + preferences: preferences, updated_by_id: 1, created_by_id: 1, ) assert(article1.preferences[:twitter]) assert_equal(1_234_567_890, article1.preferences[:twitter][:mention_ids][0]) - assert_nil(article1.preferences[:twitter][:geo]) - assert_equal(NilClass, article1.preferences[:twitter][:geo].class) - assert_nil(article1.preferences[:twitter][:place]) - assert_equal(NilClass, article1.preferences[:twitter][:place].class) + assert_equal(ActiveSupport::HashWithIndifferentAccess, article1.preferences[:twitter][:geo].class) + assert(article1.preferences[:twitter][:geo].blank?) + assert_equal(ActiveSupport::HashWithIndifferentAccess, article1.preferences[:twitter][:place].class) + assert(article1.preferences[:twitter][:place].blank?) - twitter_preferences = { + twitter_preferences = { + mention_ids: [1_234_567_890], + geo: Twitter::NullObject.new, + retweeted: false, + possibly_sensitive: false, + in_reply_to_user_id: 1_234_567_890, + place: Twitter::NullObject.new, + retweet_count: 0, + source: 'Tweetbot for Mac', + favorited: false, + truncated: false + } + preferences = TweetBase.preferences_cleanup( + twitter: twitter_preferences, + links: [ + { + url: 'https://twitter.com/statuses/123', + target: '_blank', + name: 'on Twitter', + }, + ], + ) + article2 = Ticket::Article.create!( + ticket_id: ticket1.id, + type_id: Ticket::Article::Type.find_by(name: 'twitter status').id, + sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id, + from: '@example', + body: 'some tweet', + internal: false, + preferences: preferences, + updated_by_id: 1, + created_by_id: 1, + ) + assert(article2.preferences[:twitter]) + assert_equal(1_234_567_890, article2.preferences[:twitter][:mention_ids][0]) + assert_equal(ActiveSupport::HashWithIndifferentAccess, article2.preferences[:twitter][:geo].class) + assert(article2.preferences[:twitter][:geo].blank?) + assert_equal(ActiveSupport::HashWithIndifferentAccess, article2.preferences[:twitter][:place].class) + assert(article2.preferences[:twitter][:place].blank?) + + twitter_preferences = { mention_ids: [1_234_567_890], geo: Twitter::Geo.new(coordinates: [1, 1]), retweeted: false, @@ -80,7 +120,7 @@ class TicketArticleTwitter < ActiveSupport::TestCase truncated: false } preferences = { - twitter: twitter_preferences, + twitter: TweetBase.preferences_cleanup(twitter_preferences), links: [ { url: 'https://twitter.com/statuses/123', @@ -90,23 +130,64 @@ class TicketArticleTwitter < ActiveSupport::TestCase ], } - article2 = Ticket::Article.create!( + article3 = Ticket::Article.create!( ticket_id: ticket1.id, type_id: Ticket::Article::Type.find_by(name: 'twitter status').id, sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id, from: '@example', body: 'some tweet', internal: false, - preferences: TweetBase.preferences_cleanup(preferences), + preferences: preferences, updated_by_id: 1, created_by_id: 1, ) - assert(article2.preferences[:twitter]) - assert_equal(1_234_567_890, article2.preferences[:twitter][:mention_ids][0]) - assert_equal(ActiveSupport::HashWithIndifferentAccess, article2.preferences[:twitter][:geo].class) - assert_equal({ 'coordinates' => [1, 1] }, article2.preferences[:twitter][:geo]) - assert_equal(ActiveSupport::HashWithIndifferentAccess, article2.preferences[:twitter][:place].class) - assert_equal({ 'country' => 'da', 'name' => 'do', 'woeid' => 1, 'id' => 1 }, article2.preferences[:twitter][:place]) + assert(article3.preferences[:twitter]) + assert_equal(1_234_567_890, article3.preferences[:twitter][:mention_ids][0]) + assert_equal(ActiveSupport::HashWithIndifferentAccess, article3.preferences[:twitter][:geo].class) + assert_equal({ 'coordinates' => [1, 1] }, article3.preferences[:twitter][:geo]) + assert_equal(ActiveSupport::HashWithIndifferentAccess, article3.preferences[:twitter][:place].class) + assert_equal({ 'country' => 'da', 'name' => 'do', 'woeid' => 1, 'id' => 1 }, article3.preferences[:twitter][:place]) + + twitter_preferences = { + mention_ids: [1_234_567_890], + geo: Twitter::Geo.new(coordinates: [1, 1]), + retweeted: false, + possibly_sensitive: false, + in_reply_to_user_id: 1_234_567_890, + place: Twitter::Place.new(country: 'da', name: 'do', woeid: 1, id: 1), + retweet_count: 0, + source: 'Tweetbot for Mac', + favorited: false, + truncated: false + } + preferences = TweetBase.preferences_cleanup( + twitter: twitter_preferences, + links: [ + { + url: 'https://twitter.com/statuses/123', + target: '_blank', + name: 'on Twitter', + }, + ], + ) + + article4 = Ticket::Article.create!( + ticket_id: ticket1.id, + type_id: Ticket::Article::Type.find_by(name: 'twitter status').id, + sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id, + from: '@example', + body: 'some tweet', + internal: false, + preferences: preferences, + updated_by_id: 1, + created_by_id: 1, + ) + assert(article4.preferences[:twitter]) + assert_equal(1_234_567_890, article4.preferences[:twitter][:mention_ids][0]) + assert_equal(ActiveSupport::HashWithIndifferentAccess, article4.preferences[:twitter][:geo].class) + assert_equal({ 'coordinates' => [1, 1] }, article4.preferences[:twitter][:geo]) + assert_equal(ActiveSupport::HashWithIndifferentAccess, article4.preferences[:twitter][:place].class) + assert_equal({ 'country' => 'da', 'name' => 'do', 'woeid' => 1, 'id' => 1 }, article4.preferences[:twitter][:place]) end