From 3fa9f6414d8f3ee431b62e97b396138724040bca Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Tue, 28 May 2013 19:51:57 +0300 Subject: [PATCH] Fix Twitter crossposting (#2758). Refactor some service posting related code. --- Changelog.md | 1 + app/models/service.rb | 22 ----------- app/models/services/facebook.rb | 4 -- app/models/services/twitter.rb | 53 ++++++++++++++++++++++----- spec/models/service_spec.rb | 13 ------- spec/models/services/facebook_spec.rb | 8 ---- spec/models/services/twitter_spec.rb | 21 +++++------ 7 files changed, 55 insertions(+), 67 deletions(-) diff --git a/Changelog.md b/Changelog.md index fcf745ddf..bb8bd8542 100644 --- a/Changelog.md +++ b/Changelog.md @@ -16,6 +16,7 @@ * Add back-to-top button on tag and user pages [#4185](https://github.com/diaspora/diaspora/issues/4185) * Fix reopened issue by changing the comment/post submit keyboard sortcut to ctrl+enter from shift+enter [#3897](https://github.com/diaspora/diaspora/issues/3897) * Show medium avatar in hovercard [#4203](https://github.com/diaspora/diaspora/pull/4203) +* Fix posting to Twitter [#2758](https://github.com/diaspora/diaspora/issues/2758) ## Features diff --git a/app/models/service.rb b/app/models/service.rb index 070a81732..fd5a1e215 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -13,26 +13,6 @@ class Service < ActiveRecord::Base service_strings.map{|s| "Services::#{s.titleize}"} end - def public_message(post, length, url = "", always_include_post_url = true, markdown = false) - Rails.logger.info("Posting out to #{self.class}") - if ! markdown - post_text = strip_markdown(post.text(:plain_text => true)) - else - post_text = post.text(:plain_text => true) - end - if post_text.length <= length && ! always_include_post_url - # include url to diaspora when posting only when it exceeds length - url = "" - space_for_url = 0 - else - url = " " + Rails.application.routes.url_helpers.short_post_url(post, :protocol => AppConfig.pod_uri.scheme, :host => AppConfig.pod_uri.authority) - space_for_url = 21 + 1 - end - truncated = truncate(post_text, :length => (length - space_for_url)) - truncated = "#{truncated}#{url}" - return truncated - end - def profile_photo_url nil end @@ -42,5 +22,3 @@ class Service < ActiveRecord::Base end end -require 'services/facebook' -require 'services/twitter' diff --git a/app/models/services/facebook.rb b/app/models/services/facebook.rb index 83d4da631..7eef3470c 100644 --- a/app/models/services/facebook.rb +++ b/app/models/services/facebook.rb @@ -29,10 +29,6 @@ class Services::Facebook < Service {:message => message, :access_token => self.access_token, :link => URI.extract(message, ['https', 'http']).first} end - def public_message(post, url) - super(post, MAX_CHARACTERS, url) - end - def profile_photo_url "https://graph.facebook.com/#{self.uid}/picture?type=large&access_token=#{URI.escape(self.access_token)}" end diff --git a/app/models/services/twitter.rb b/app/models/services/twitter.rb index a640f6f5c..0df15d9d8 100644 --- a/app/models/services/twitter.rb +++ b/app/models/services/twitter.rb @@ -1,4 +1,7 @@ class Services::Twitter < Service + include ActionView::Helpers::TextHelper + include MarkdownifyHelper + MAX_CHARACTERS = 140 SHORTENED_URL_LENGTH = 21 @@ -8,21 +11,53 @@ class Services::Twitter < Service def post(post, url='') Rails.logger.debug("event=post_to_service type=twitter sender_id=#{self.user_id}") - message = public_message(post, url) - tweet = client.update(message) - post.tweet_id = tweet.id + (0...20).each do |retry_count| + begin + message = build_twitter_post(post, url, retry_count) + @tweet = client.update(message) + break + rescue Twitter::Error::Forbidden => e + if e.message != 'Status is over 140 characters' || retry_count == 20 + raise e + end + end + end + post.tweet_id = @tweet.id post.save end - - def public_message(post, url) - buffer_amt = 0 - URI.extract( post.text(:plain_text => true), ['http','https'] ) do |a_url| - buffer_amt += (a_url.length - SHORTENED_URL_LENGTH) + def adjust_length_for_urls(post_text) + real_length = post_text.length + URI.extract( post_text, ['http','https'] ) do |a_url| + # add or subtract from real length - urls for tweets are always + # shortened to SHORTENED_URL_LENGTH + if a_url.length >= SHORTENED_URL_LENGTH + real_length -= a_url.length - SHORTENED_URL_LENGTH + else + real_length += SHORTENED_URL_LENGTH - a_url.length + end end + return real_length + end + def add_post_link(post, post_text, maxchars) + post_url = Rails.application.routes.url_helpers.short_post_url( + post, + :protocol => AppConfig.pod_uri.scheme, + :host => AppConfig.pod_uri.authority + ) + truncated = truncate(post_text, :length => (maxchars - (SHORTENED_URL_LENGTH+1) )) + post_text = "#{truncated} #{post_url}" + end + + def build_twitter_post(post, url, retry_count=0) + maxchars = MAX_CHARACTERS - retry_count*5 + post_text = strip_markdown(post.text(:plain_text => true)) #if photos, always include url, otherwise not for short posts - super(post, MAX_CHARACTERS + buffer_amt, url, post.photos.any?) + if adjust_length_for_urls(post_text) > maxchars || post.photos.any? + post_text = add_post_link(post, post_text, maxchars) + end + return post_text end def profile_photo_url diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 0d5a27fb6..49d2a72e0 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -22,17 +22,4 @@ describe Service do Service.new.profile_photo_url.should be_nil end - it 'removes text formatting markdown from post text' do - service = Service.new - message = "Text with some **bolded** and _italic_ parts." - post = stub(:text => message) - service.public_message(post, 200, '', false).should match "Text with some bolded and italic parts." - end - - it 'keeps markdown in post text when specified' do - service = Service.new - message = "Text with some **bolded** and _italic_ parts." - post = stub(:text => message) - service.public_message(post, 200, '', false, true).should match 'Text with some \*\*bolded\*\* and _italic_ parts.' - end end diff --git a/spec/models/services/facebook_spec.rb b/spec/models/services/facebook_spec.rb index c283ffdb1..698a3eae9 100644 --- a/spec/models/services/facebook_spec.rb +++ b/spec/models/services/facebook_spec.rb @@ -23,14 +23,6 @@ describe Services::Facebook do to_raise(StandardError) @service.post(@post) end - - it 'should call public message' do - stub_request(:post, "https://graph.facebook.com/me/feed"). - to_return(:status => 200, :body => '{"id": "12345"}', :headers => {}) - url = "foo" - @service.should_not_receive(:public_message) - @service.post(@post, url) - end it 'removes text formatting markdown from post text' do message = "Text with some **bolded** and _italic_ parts." diff --git a/spec/models/services/twitter_spec.rb b/spec/models/services/twitter_spec.rb index 2fe9adbea..d63f3ad69 100644 --- a/spec/models/services/twitter_spec.rb +++ b/spec/models/services/twitter_spec.rb @@ -31,16 +31,16 @@ describe Services::Twitter do @service.post(@post) end - it 'should call public message' do + it 'should call build_twitter_post' do url = "foo" - @service.should_receive(:public_message).with(@post, url) + @service.should_receive(:build_twitter_post).with(@post, url, 0) @service.post(@post, url) end it 'removes text formatting markdown from post text' do message = "Text with some **bolded** and _italic_ parts." post = stub(:text => message, :photos => []) - @service.public_message(post, '').should match "Text with some bolded and italic parts." + @service.build_twitter_post(post, '').should match "Text with some bolded and italic parts." end end @@ -54,21 +54,20 @@ describe Services::Twitter do it "should not truncate a short message" do short_message = SecureRandom.hex(20) short_post = stub(:text => short_message, :photos => []) - @service.public_message(short_post, '').should match short_message + @service.build_twitter_post(short_post, '').should match short_message end it "should truncate a long message" do long_message = SecureRandom.hex(220) long_post = stub(:text => long_message, :id => 1, :photos => []) - @service.public_message(long_post, '').should match long_message.first(100) - + @service.build_twitter_post(long_post, '').length.should be < long_message.length end it "should not truncate a long message with an http url" do long_message = " http://joindiaspora.com/a-very-long-url-name-that-will-be-shortened.html " + @long_message_end long_post = stub(:text => long_message, :id => 1, :photos => []) @post.text = long_message - answer = @service.public_message(@post, '') + answer = @service.build_twitter_post(@post, '') answer.should_not match /\.\.\./ end @@ -76,14 +75,14 @@ describe Services::Twitter do it "should not truncate a long message with an https url" do long_message = " https://joindiaspora.com/a-very-long-url-name-that-will-be-shortened.html " + @long_message_end @post.text = long_message - answer = @service.public_message(@post, '') + answer = @service.build_twitter_post(@post, '') answer.should_not match /\.\.\./ end it "should truncate a long message with an ftp url" do long_message = @long_message_start + " ftp://joindiaspora.com/a-very-long-url-name-that-will-be-shortened.html " + @long_message_end long_post = stub(:text => long_message, :id => 1, :photos => []) - answer = @service.public_message(long_post, '') + answer = @service.build_twitter_post(long_post, '') answer.should match /\.\.\./ end @@ -91,7 +90,7 @@ describe Services::Twitter do it "should not truncate a message of maximum length" do exact_size_message = SecureRandom.hex(70) exact_size_post = stub(:text => exact_size_message, :id => 1, :photos => []) - answer = @service.public_message(exact_size_post, '') + answer = @service.build_twitter_post(exact_size_post, '') answer.should match exact_size_message end @@ -113,7 +112,7 @@ describe Services::Twitter do end it "should include post url in short message with photos" do - answer = @service.public_message(@status_message, '') + answer = @service.build_twitter_post(@status_message, '') answer.should include 'http' end