Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Correct retweet response. Retweeted status should be the original tweet #541

Merged
merged 1 commit into from

4 participants

achiinto Coveralls Erik Michaels-Ober Ian Choi
achiinto

I am suggesting to revert the fix for issuse #228. As opposed to Issue #228, the Twitter api response with original tweet as the retweeted_status is actually a correct behaviour. Similarly, all statuses list endpoints would consistently return the original tweet as the retweeted_status, whenever a retweet is encountered.

However, the fix for issue #228 somehow reversed the response from Twitter API for POST retweet, making the gem behaviour inconsistent when encountering a retweet.

Example,
Twitter API:

  • GET statuses/user_timeline -> return original tweet as retweeted_status whenever a retweet is encountered.
  • POST statuses/retweet/:id -> return original tweet as retweeted_status, nested in the new retweet created.

Twitter Gem:

  • GET statuses/user_timeline -> return original tweet as retweeted_status whenever a retweet is encountered.
  • POST statuses/retweet/:id -> return new retweet as retweeted_status, nested in the the original tweet.

Also, the name, "retweeted_status", implies the "status being retweeted", thus the fix on issue #228 is wrong as the developer who fixed #228 mistaken "retweeted_status" as "the status generated due to taking a retweet action". This might also suggests Twitter::Tweet with alias_method for retweet as retweeted_status is also wrong.

The twitter documentation, https://dev.twitter.com/docs/api/1.1/post/statuses/retweet/:id, has example demonstrating that original tweet is the retweeted_status. (Although the summary of the api is confusing, which I will file a bug to api.)

Also, on a side note, Issue #228 thought that Twitter API has a bug and implemented a fix at the Gem level. I think this is a bad practice. If we think the API is faulty, we should file a bug to Twitter instead.

Coveralls

Coverage Status

Coverage decreased (-0.03%) when pulling b6c4e65 on jugnoo:master into 3c0a2db on sferik:master.

achiinto

also documentation, https://dev.twitter.com/docs/platform-objects/tweets, describe retweeted_status as

"Users can amplify the broadcast of tweets authored by other users by retweeting. Retweets can be distinguished from typical Tweets by the existence of a retweeted_status attribute. This attribute contains a representation of the original Tweet that was retweeted. Note that retweets of retweets do not show representations of the intermediary retweet, but only the original tweet. (Users can also unretweet a retweet they created by deleting their retweet.)"

Erik Michaels-Ober
Owner

Thanks for opening this issue. I agree with your logic and am surprised I merged #228 without any discussion. I also agree that the Twitter::Tweet#retweet alias should be removed. Can you please add that change to this pull request?

achiinto

updated.

Erik Michaels-Ober sferik merged commit ca6093a into from
Erik Michaels-Ober
Owner

Thanks again!

Coveralls

Coverage Status

Coverage decreased (-0.03%) when pulling 70fede7 on jugnoo:master into 3c0a2db on sferik:master.

Erik Michaels-Ober sferik referenced this pull request from a commit in sferik/t
Erik Michaels-Ober Update retweeted status for sferik/twitter#541 d277892
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
4 lib/twitter/rest/tweets.rb
View
@@ -300,9 +300,7 @@ def parallel_tweets_from_response(request_method, path, args)
def post_retweet(tweet, options)
response = post("/1.1/statuses/retweet/#{extract_id(tweet)}.json", options).body
- retweeted_status = response.delete(:retweeted_status)
- retweeted_status[:retweeted_status] = response
- Twitter::Tweet.new(retweeted_status)
+ Twitter::Tweet.new(response)
end
end
end
1  lib/twitter/tweet.rb
View
@@ -19,7 +19,6 @@ class Tweet < Twitter::Identity
object_attr_reader :Metadata, :metadata
object_attr_reader :Place, :place
object_attr_reader :Tweet, :retweeted_status
- alias_method :retweet, :retweeted_status
alias_method :retweeted_tweet, :retweeted_status
alias_method :retweet?, :retweeted_status?
alias_method :retweeted_tweet?, :retweeted_status?
8 spec/twitter/rest/tweets_spec.rb
View
@@ -347,8 +347,8 @@
tweets = @client.retweet(25_938_088_801)
expect(tweets).to be_an Array
expect(tweets.first).to be_a Twitter::Tweet
- expect(tweets.first.text).to eq("As for the Series, I'm for the Giants. Fuck Texas, fuck Nolan Ryan, fuck George Bush.")
- expect(tweets.first.retweeted_tweet.text).to eq("RT @gruber: As for the Series, I'm for the Giants. Fuck Texas, fuck Nolan Ryan, fuck George Bush.")
+ expect(tweets.first.text).to eq("RT @gruber: As for the Series, I'm for the Giants. Fuck Texas, fuck Nolan Ryan, fuck George Bush.")
+ expect(tweets.first.retweeted_tweet.text).to eq("As for the Series, I'm for the Giants. Fuck Texas, fuck Nolan Ryan, fuck George Bush.")
expect(tweets.first.retweeted_tweet.id).not_to eq(tweets.first.id)
end
context 'already retweeted' do
@@ -393,8 +393,8 @@
tweets = @client.retweet!(25_938_088_801)
expect(tweets).to be_an Array
expect(tweets.first).to be_a Twitter::Tweet
- expect(tweets.first.text).to eq("As for the Series, I'm for the Giants. Fuck Texas, fuck Nolan Ryan, fuck George Bush.")
- expect(tweets.first.retweeted_tweet.text).to eq("RT @gruber: As for the Series, I'm for the Giants. Fuck Texas, fuck Nolan Ryan, fuck George Bush.")
+ expect(tweets.first.text).to eq("RT @gruber: As for the Series, I'm for the Giants. Fuck Texas, fuck Nolan Ryan, fuck George Bush.")
+ expect(tweets.first.retweeted_tweet.text).to eq("As for the Series, I'm for the Giants. Fuck Texas, fuck Nolan Ryan, fuck George Bush.")
expect(tweets.first.retweeted_tweet.id).not_to eq(tweets.first.id)
end
context 'forbidden' do
Something went wrong with that request. Please try again.