Skip to content

Convert list.rb, list_members.rb, and list_subscribers.rb to new Twitter API #154

Closed
wants to merge 15 commits into from

3 participants

@erebor
Collaborator
erebor commented Apr 10, 2011

With @sferik's very patient help (and a boost from @alindeman), I've converted the list methods to use the new Twitter API. In addition, I added or fixed all the places I could find where it's possible to pass in either a numeric id (list or user) or a string (slug or screen_name), and updated docs and specs to reflect that.

I've not done this before (submit changes to an existing public project), so I'm not sure if this is the proper way to do the pull request.

But it's late and I'm done and I want to send it off!

@sferik sferik and 1 other commented on an outdated diff Apr 10, 2011
lib/twitter/client/list.rb
- # @param options [Hash] A customizable set of options.
- # @option options [String] :mode ('public') Whether your list is public or private. Values can be 'public' or 'private'.
- # @option options [String] :description The description to give the list.
- # @example Update the "presidents" list to have the description "Presidents of the United States of America"
- # Twitter.list_update("sferik", "presidents", :description => "Presidents of the United States of America")
+ # @overload list_update(user, list, options={})
+ # @param user [Integer, String] A Twitter user ID or screen name.
+ # @param list [Integer, String] The list_id or slug for the list.
+ # @param options [Hash] A customizable set of options.
+ # @option options [String] :mode ('public') Whether your list is public or private. Values can be 'public' or 'private'.
+ # @option options [String] :description The description to give the list.
+ # @example Update the "presidents" list to have the description "Presidents of the United States of America"
+ # Twitter.list_update("sferik", "presidents", :description => "Presidents of the United States of America")
+ # Twitter.list_update(7505382, "presidents", :description => "Presidents of the United States of America")
+ # Twitter.list_update("sferik", 8863586, :description => "Presidents of the United States of America")
+ # Twitter.list_update(7505382, 8863586, :description => "Presidents of the United States of America")
@sferik
Owner
sferik added a note Apr 10, 2011

Since this method is overloaded, there should be another set of documentation showing how to call it without the first argument. It's worth noting that the documentation was incomplete when you started—and you've made it no worse—but it's still incomplete.

@erebor
Collaborator
erebor added a note Apr 10, 2011

Fixed, I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sferik sferik commented on the diff Apr 10, 2011
lib/twitter/client/list.rb
# @format :json, :xml
# @authenticated true
# @rate_limited true
def lists(*args)
options = {:cursor => -1}.merge(args.last.is_a?(Hash) ? args.pop : {})
- screen_name = args.first
- if screen_name
- response = get("#{screen_name}/lists", options)
- else
- response = get('lists', options)
- end
+ user = args.first
+ merge_user_into_options!(user, options) if user
+ response = get("lists", options)
@sferik
Owner
sferik added a note Apr 10, 2011

It looks like you removed the ability to not pass in a user, both from the documentation and from the code itself. This interface needs to be maintained.

@erebor
Collaborator
erebor added a note Apr 10, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sferik sferik commented on the diff Apr 10, 2011
lib/twitter/client/list_subscribers.rb
format.to_s.downcase == 'xml' ? response['users_list'] : response
end
- # Unsubscribes the authenticated user form the specified list
+ # Make the authenticated user follow the specified list
@sferik
Owner
sferik added a note Apr 10, 2011

Nice catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sferik sferik commented on the diff Apr 10, 2011
lib/twitter/client/list_subscribers.rb
format.to_s.downcase == 'xml' ? response['list'] : response
end
- # Make the authenticated user follow the specified list
+ # Unsubscribes the authenticated user form the specified list
@sferik
Owner
sferik added a note Apr 10, 2011

Funny that this was backwards.

@erebor
Collaborator
erebor added a note Apr 10, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sferik sferik commented on the diff Apr 10, 2011
lib/twitter/client/list_subscribers.rb
true
- rescue Twitter::NotFound
+ rescue Twitter::NotFound, Twitter::Forbidden
@sferik
Owner
sferik added a note Apr 10, 2011

I assume Twitter throws a 403 when the user is protected? This is a nice addition.

@erebor
Collaborator
erebor added a note Apr 10, 2011

I don't know about protected, but if you pass in a user id that doesn't exist at all, it throws a 403, so I thought it better to return false in that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sferik
Owner
sferik commented Apr 10, 2011

Thanks for making these changes. Pulling it into the upstream master now.

@sferik sferik closed this Apr 10, 2011
@alindeman

Nice work @erebor. Glad to give a boost :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.