Merge pull request #4187 from jaywink/2758-twitter-crossposting

Fix Twitter crossposting by correctly calculating URL shortening

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
 

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'

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

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)
+    (0...20).each do |retry_count|
-    tweet = client.update(message)
+      begin
-    post.tweet_id = tweet.id
+        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 adjust_length_for_urls(post_text)
-  def public_message(post, url)
+    real_length = post_text.length
-    buffer_amt = 0
+    URI.extract( post_text, ['http','https'] ) do |a_url|
-    URI.extract( post.text(:plain_text => true), ['http','https'] ) do |a_url|
+      # add or subtract from real length - urls for tweets are always 
-      buffer_amt += (a_url.length - SHORTENED_URL_LENGTH)
+      # 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

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

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."

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