Fixes #2715 - Twitter status URLs are broken

This commit is a complement of its parent:
the parent fixes broken links to Twitter DMs,
while this one fixes broken links to Twitter statuses.

When importing tweets (articles of type "twitter status"),
Zammad generates a source link URL using the following format:

    https://twitter.com/statuses/:id

and then stores that URL under `article.preferences[:links]`.

This URL template worked until as recently as 2017[0],
but currently fails for users actively logged in to Twitter.
Now, the correct URL template appears to be

    https://twitter.com/:user_id/status/:id

where `:user_id` is not strict, and may be any word (\w+) <= 20 chars.
Try it yourself:

    $ curl https://twitter.com/elonmusk/status/1069382411899817990
    <html><body>You are being <a href="https://twitter.com/medenhofer/status/1069382411899817990">redirected</a>.</body></html>

In this commit, we replace `:user_id` with a single underscore (`_`).
This behavior is not officially documented anywhere (as far as I know),
but it works (for now).

This commit also extends the previous commit's DB migration/bg job
to rectify existing, broken tweet URLs stored in the database.
For performance purposes, this migration is performed in the background
and limited to the latest 10,000 Twitter articles.

[0]: https://stackoverflow.com/questions/41786123
GitHub: https://github.com/zammad/zammad/issues/2715
This commit is contained in:
Ryan Lue 2019-11-14 14:03:34 +08:00 committed by Thorsten Eckel
parent 147cbc29b8
commit 25cd644951
7 changed files with 44 additions and 16 deletions

View file

@ -1,15 +1,29 @@
class Issue2715FixBrokenTwitterUrlsJob < ApplicationJob class Issue2715FixBrokenTwitterUrlsJob < ApplicationJob
STATUS_TEMPLATE = 'https://twitter.com/_/status/%<message_id>s'.freeze
DM_TEMPLATE = 'https://twitter.com/messages/%<recipient_id>s-%<sender_id>s'.freeze
def perform def perform
Ticket::Article.joins(:type) Ticket::Article.joins(:type)
.where(ticket_article_types: { name: 'twitter direct-message' }) .where(ticket_article_types: { name: ['twitter status', 'twitter direct-message'] })
.order(created_at: :desc) .order(created_at: :desc)
.limit(10_000) .limit(10_000)
.find_each do |dm| .find_each(&method(:fix_broken_links))
dm.preferences[:links]&.each do |link|
link[:url] = "https://twitter.com/messages/#{dm.preferences[:twitter][:recipient_id]}-#{dm.preferences[:twitter][:sender_id]}"
end end
dm.save! private
def fix_broken_links(article)
type = Ticket::Article::Type.lookup(id: article.type_id).name
article.preferences[:links]&.each do |link|
link[:url] = case type
when 'twitter status'
STATUS_TEMPLATE % article.attributes.symbolize_keys
when 'twitter direct-message'
DM_TEMPLATE % article.preferences[:twitter].symbolize_keys
end end
end end
article.save!
end
end end

View file

@ -105,7 +105,7 @@ class Observer::Ticket::Article::CommunicateTwitter::BackgroundJob
article.message_id = tweet.id.to_s article.message_id = tweet.id.to_s
article.preferences['links'] = [ article.preferences['links'] = [
{ {
url: "https://twitter.com/statuses/#{tweet.id}", url: TwitterSync::STATUS_URL_TEMPLATE % tweet.id,
target: '_blank', target: '_blank',
name: 'on Twitter', name: 'on Twitter',
}, },

View file

@ -4,6 +4,8 @@ require 'http/uri'
class TwitterSync class TwitterSync
STATUS_URL_TEMPLATE = 'https://twitter.com/_/status/%s'.freeze
attr_accessor :client attr_accessor :client
def initialize(auth, payload = nil) def initialize(auth, payload = nil)
@ -303,7 +305,7 @@ class TwitterSync
twitter: self.class.preferences_cleanup(twitter_preferences), twitter: self.class.preferences_cleanup(twitter_preferences),
links: [ links: [
{ {
url: "https://twitter.com/statuses/#{item['id']}", url: STATUS_URL_TEMPLATE % item['id'],
target: '_blank', target: '_blank',
name: 'on Twitter', name: 'on Twitter',
}, },
@ -399,7 +401,7 @@ class TwitterSync
twitter: self.class.preferences_cleanup(twitter_preferences), twitter: self.class.preferences_cleanup(twitter_preferences),
links: [ links: [
{ {
url: "https://twitter.com/statuses/#{tweet.id}", url: STATUS_URL_TEMPLATE % tweet.id,
target: '_blank', target: '_blank',
name: 'on Twitter', name: 'on Twitter',
}, },
@ -571,7 +573,7 @@ create a tweet ot direct message from an article
twitter: twitter_preferences, twitter: twitter_preferences,
links: [ links: [
{ {
url: 'https://twitter.com/statuses/123', url: 'https://twitter.com/_/status/123',
target: '_blank', target: '_blank',
name: 'on Twitter', name: 'on Twitter',
}, },
@ -584,7 +586,7 @@ or
twitter: TwitterSync.preferences_cleanup(twitter_preferences), twitter: TwitterSync.preferences_cleanup(twitter_preferences),
links: [ links: [
{ {
url: 'https://twitter.com/statuses/123', url: 'https://twitter.com/_/status/123',
target: '_blank', target: '_blank',
name: 'on Twitter', name: 'on Twitter',
}, },

View file

@ -23,6 +23,7 @@ FactoryBot.define do
end end
association :ticket, factory: :twitter_ticket association :ticket, factory: :twitter_ticket
message_id { '775410014383026176' }
body { Faker::Lorem.sentence } body { Faker::Lorem.sentence }
end end

View file

@ -3,11 +3,18 @@ require_dependency 'issue_2715_fix_broken_twitter_urls_job' # Rails autoloading
RSpec.describe Issue2715FixBrokenTwitterUrlsJob, type: :job do RSpec.describe Issue2715FixBrokenTwitterUrlsJob, type: :job do
context 'with existing Twitter articles' do context 'with existing Twitter articles' do
let!(:tweet) { create(:twitter_article, preferences: tweet_preferences) }
let!(:dm) { create(:twitter_dm_article, preferences: dm_preferences) } let!(:dm) { create(:twitter_dm_article, preferences: dm_preferences) }
let(:tweet_preferences) do
# NOTE: Faker 2.0+ has deprecated the `#number(20)` syntax in favor of `#number(digits: 20)`.
{ links: [{ url: "https://twitter.com/statuses/#{Faker::Number.number(20)}" }] }
end
let(:dm_preferences) do let(:dm_preferences) do
{ {
links: Array.new(5, &link_hash), # NOTE: Faker 2.0+ has deprecated the `#number(20)` syntax in favor of `#number(digits: 20)`.
links: [{ url: "https://twitter.com/statuses/#{Faker::Number.number(20)}" }],
twitter: { twitter: {
recipient_id: recipient_id, recipient_id: recipient_id,
sender_id: sender_id, sender_id: sender_id,
@ -15,11 +22,15 @@ RSpec.describe Issue2715FixBrokenTwitterUrlsJob, type: :job do
} }
end end
# NOTE: Faker 2.0+ has deprecated the `#number(20)` syntax in favor of `#number(digits: 20)`.
let(:link_hash) { ->(_) { { url: "https://twitter.com/statuses/#{Faker::Number.number(20)}" } } }
let(:recipient_id) { '1234567890' } let(:recipient_id) { '1234567890' }
let(:sender_id) { '0987654321' } let(:sender_id) { '0987654321' }
it 'reformats all Twitter status URLs' do
expect { described_class.perform_now }
.to change { urls_of(tweet) }
.to all(match(%r{^https://twitter.com/_/status/#{tweet.message_id}$}))
end
it 'reformats all Twitter DM URLs' do it 'reformats all Twitter DM URLs' do
expect { described_class.perform_now } expect { described_class.perform_now }
.to change { urls_of(dm) } .to change { urls_of(dm) }

View file

@ -317,7 +317,7 @@ RSpec.describe Ticket::Article, type: :model do
.to('') # Tweet in VCR cassette is addressed to no one .to('') # Tweet in VCR cassette is addressed to no one
end end
it 'sets #message_id to tweet ID (https://twitter.com/statuses/<id>)' do it 'sets #message_id to tweet ID (https://twitter.com/_/status/<id>)' do
expect(&run_bg_jobs) expect(&run_bg_jobs)
.to change { twitter_article.reload.message_id } .to change { twitter_article.reload.message_id }
.to('1069382411899817990') .to('1069382411899817990')
@ -332,7 +332,7 @@ RSpec.describe Ticket::Article, type: :model do
.to include( .to include(
'name' => 'on Twitter', 'name' => 'on Twitter',
'target' => '_blank', 'target' => '_blank',
'url' => "https://twitter.com/statuses/#{twitter_article.message_id}" 'url' => "https://twitter.com/_/status/#{twitter_article.message_id}"
) )
end end