Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add favorite support to tvdb_party #6

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add favorite support to tvdb_party #6

wants to merge 4 commits into from

Conversation

marcbowes
Copy link

The attached commits add favorite support to tvdb_party.

Client is imbued with the following new methods:

get_favorites(account_id)
add_favorite(account_id, series_id)
remove_favorite(account_id, series_id)

Series gets the following convenience methods:

is_favorite?
favorite!(account_id)
unfavorite!(account_id)

There is some sort of test attempt, but it doesn't really test much as their API doesn't offer much in the way of response validation and I didn't want to waste time stubbing HTTP reqs.

Additionally, I updated the README to reflect the new changes.

@maddox
Copy link
Owner

maddox commented Apr 27, 2011

I'm not too sure about this api. Passing in the account id into the methods on the fly like that isn't ideal. Ideally, you'd pass it in when creating the client, like you do with the api key.

That being said, this lib is more for getting tv meta about series and way less about personal use on the tvdb site. Changing it to allow the user id to be passed in on init just to get favorites doesn't outweigh the cost of changing the init. Thanks!

@maddox maddox closed this Apr 27, 2011
@marcbowes
Copy link
Author

Reopening to have you take a second, more in-depth look and avoid me having to maintain a separate fork just for this feature. Here are my motivations why this should be included:

  • Client libraries don't have control over how the API is implemented. Just because their API isn't 'ideal', shouldn't exclude support in a client library.
  • Passing the account id on init is the wrong way to implement this as it is not reflective of their API. The favorites API doesn't restrict you to viewing/modifying favorites for the account linked to the API key: you can invoke it against any account id you know about. Thus, the client implementation should mimic the functionality on the server.
  • Having favorites is a useful feature exactly for finding this meta. If you want to find the next air date of a show of yours, you need the show id. I don't know if you've noticed, but show names (and tvdb's search support) is non-deterministic. Favorites allow you to authoritatively build a list of show id's you care about without having to keep that logic on the client side.

Again, this is a tvdb client and this pull request is a one-to-one implementation of how their API works. Please reconsider :-)

@marcbowes marcbowes reopened this Apr 27, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants