From 39d57f3428cc53add0ef510a1ce39079734c8ee7 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Thu, 28 Jun 2018 17:17:27 +0200 Subject: [PATCH] Follow up for issue #1394 - added Twitter::Geo as object to convert - Reindex elastic search not possible because of /Twitter::NullObject. --- ...xed_twitter_ticket_article_preferences5.rb | 36 ++++++++ lib/tweet_base.rb | 2 +- test/unit/ticket_article_twitter_test.rb | 87 ++++++++++++++----- 3 files changed, 103 insertions(+), 22 deletions(-) create mode 100644 db/migrate/20180628000001_fixed_twitter_ticket_article_preferences5.rb diff --git a/db/migrate/20180628000001_fixed_twitter_ticket_article_preferences5.rb b/db/migrate/20180628000001_fixed_twitter_ticket_article_preferences5.rb new file mode 100644 index 000000000..59634974b --- /dev/null +++ b/db/migrate/20180628000001_fixed_twitter_ticket_article_preferences5.rb @@ -0,0 +1,36 @@ +class FixedTwitterTicketArticlePreferences5 < ActiveRecord::Migration[5.0] + def up + + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + # find article preferences with Twitter::NullObject and replace it with nill to prevent elasticsearch index issue + article_type_ids = Ticket::Article::Type.where(name: ['twitter status', 'twitter direct-message']).pluck(:id) + article_ids = Ticket::Article.where(type_id: article_type_ids).pluck(:id) + article_ids.each do |article_id| + article = Ticket::Article.find(article_id) + next if !article.preferences + changed = false + article.preferences.each_value do |value| + next if value.class != ActiveSupport::HashWithIndifferentAccess + value.each do |sub_key, sub_level| + if sub_level.class == NilClass + value[sub_key] = nil + next + end + if sub_level.class == Twitter::Place || sub_level.class == Twitter::Geo + value[sub_key] = sub_level.attrs + changed = true + next + end + next if sub_level.class != Twitter::NullObject + value[sub_key] = nil + changed = true + end + end + next if !changed + article.save! + end + + end +end diff --git a/lib/tweet_base.rb b/lib/tweet_base.rb index 49d3436a1..c40a5b839 100644 --- a/lib/tweet_base.rb +++ b/lib/tweet_base.rb @@ -373,7 +373,7 @@ class TweetBase value[sub_key] = nil next end - if sub_level.class == Twitter::Place + if sub_level.class == Twitter::Place || sub_level.class == Twitter::Geo value[sub_key] = sub_level.attrs next end diff --git a/test/unit/ticket_article_twitter_test.rb b/test/unit/ticket_article_twitter_test.rb index d76f9bf11..eeeb85bd1 100644 --- a/test/unit/ticket_article_twitter_test.rb +++ b/test/unit/ticket_article_twitter_test.rb @@ -28,6 +28,28 @@ class TicketArticleTwitter < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) + 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 = { + twitter: twitter_preferences, + links: [ + { + url: 'https://twitter.com/statuses/123', + target: '_blank', + name: 'on Twitter', + }, + ], + } article1 = Ticket::Article.create!( ticket_id: ticket1.id, type_id: Ticket::Article::Type.find_by(name: 'twitter status').id, @@ -35,27 +57,7 @@ class TicketArticleTwitter < ActiveSupport::TestCase from: '@example', body: 'some tweet', internal: false, - preferences: TweetBase.new.preferences_cleanup(ActiveSupport::HashWithIndifferentAccess.new( - twitter: { - '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 - }, - links: [ - { - url: 'https://twitter.com/statuses/123', - target: '_blank', - name: 'on Twitter', - }, - ], - )), + preferences: TweetBase.new.preferences_cleanup(preferences), updated_by_id: 1, created_by_id: 1, ) @@ -63,6 +65,49 @@ class TicketArticleTwitter < ActiveSupport::TestCase 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) + + 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 = { + 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: TweetBase.new.preferences_cleanup(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]) end