Allow users lookup to optionally be handled via a GET request #330

Merged
merged 3 commits into from Nov 30, 2012

Conversation

Projects
None yet
2 participants
Contributor

thomasklemm commented Nov 29, 2012

Hey Erik, hey everyone who reads this PR :-D,

thanks for making and maintaining this great gem!

I'm in the process of making an app that allows users to uncover who started or stopped following them.

All requests im currently using except the users/lookup use GET requests, which only require 'Read' permission for the application.

Not wanting my users to grant me write access when I actually do not want to post any information, this PR implements the option to optionally make a GET request for the users/lookup resource, which is in fact the only one I found where this option makes some difference.

Since Twitter encourages making POST requests to this users/lookup resource in case of requesting up to 100 screen_names or user_ids, the POST request should remain the standard.

Twitter.users('sferik', 'thomasjklemm') #  => POST request
Twitter.users('sferik', 'thomasjklemm', method: :get) # => GET request 

More on Twitter's application permission model.

Please feel free to suggest changes in the implementation, I'll try to respond quickly.

Best regards,
Thomas

Owner

sferik commented Nov 29, 2012

@thomasklemm: Thank you for this patch and detailed pull request.

@brucew originally reported this issue here: #324. Shortly thereafter, I reported it to Twitter: https://dev.twitter.com/issues/659. I still believe this is a bug in the Twitter API but I'm happy to merge in this workaround until they fix it, which I'm pessimistic about.

The only change I'd ask you to make is to add the optional method parameter to the method's YARD-formatted comment. This comment block automatically generates HTML documentation at http://rdoc.info/gems/twitter/Twitter/API#users-instance_method. This way, users of the gem will be able to discover this awesome feature without needing to dive into the source code.

Thanks again!

Owner

sferik commented Nov 29, 2012

Note: If you make that documentation change and push it to your branch, it will automatically be appended to this pull request.

Contributor

thomasklemm commented Nov 30, 2012

Finally found a way to preview the documentation changes with yard server --reload. I hope you like them.

@sferik sferik added a commit that referenced this pull request Nov 30, 2012

@sferik sferik Merge pull request #330 from thomasklemm/users_lookup_get_request
Allow users lookup to optionally be handled via a GET request
595e423

@sferik sferik merged commit 595e423 into sferik:master Nov 30, 2012

Contributor

thomasklemm commented Nov 30, 2012

Thanks for your feedback and speed 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment