TheGiftsProject / omniauth-ebay

OmniAuth Strategy for Open eBay Apps
12 stars 16 forks source link

Add ability to use sandbox OAuth credentials to authenticate to EBay. #8

Closed rpocklin closed 10 years ago

rpocklin commented 10 years ago

I've made the environment optional, defaulting to production, so that users of this gem can test their code in the sandbox before using it in a production environment. Note there is a change to the arguments in the omniauth initializer - I didn't think passing the API url was all that useful and should be derived from the environment flag with this change. AFAIK the EBay XML API doesn't really change.

Tests and docs have been updated in line with code changes, let me know what you think.

jfelchner commented 10 years ago

I :heart: this change. Why has this not been merged in yet?

itayadler commented 10 years ago

This feature exists in version 0.9.4, you can now provide the loginurl when initialising the strategy.

jfelchner commented 10 years ago

@itayadler that's not the point, the point is that the login URL is pretty much set by eBay, so why should the user have to know what it is? In this PR, setting the environment to "sandbox" or "production" changes both the login URL and the API URL.

And, as far as I can tell, the API URL cannot be specified from the options, even in 0.9.4.

itayadler commented 10 years ago

It can using the apiurl option. Ur right it does make more sense to specify the environment than to provide the URLs.. On Apr 18, 2014 10:39 PM, "Jeff Felchner" notifications@github.com wrote:

@itayadler https://github.com/itayadler that's not the point, the point is that the login URL is pretty much set by eBay, so why should the user have to know what it is? In this PR, setting the environment to "sandbox" or "production" changes both the login URL and the API URL.

And, as far as I can tell, the API URL cannot be specified from the options, even in 0.9.4.

— Reply to this email directly or view it on GitHubhttps://github.com/TheGiftsProject/omniauth-ebay/pull/8#issuecomment-40837944 .

rpocklin commented 10 years ago

Is there a reason you don't want to accept this PR?

It makes it easier to use and it is 4 EBay API URLs you can't get wrong now.

I've found the Ebay docs pretty average in some areas which is why I think the PR adds value.

jfelchner commented 10 years ago

Completely agree, the eBay API docs are awful. It took me 15 minutes just to find the right URLs for the sandbox environment.

itayadler commented 10 years ago

@rpocklin Please rebase it against the latest master and change is_sandbox? method to sandbox? and I'll release a new version with this change.

jfelchner commented 10 years ago

Awesome news! Thanks @itayadler and @rpocklin!!! :smile:

rpocklin commented 10 years ago

@itayadler Done.

itayadler commented 10 years ago

Thanks for the PR :+1: