MrSwitch / hello.js

A Javascript RESTFUL API library for connecting with OAuth2 services, such as Google+ API, Facebook Graph and Windows Live Connect
https://adodson.com/hello.js/
MIT License
4.63k stars 548 forks source link

Flickr module #5

Closed Pasvaz closed 11 years ago

Pasvaz commented 11 years ago

I'm trying to add the Flickr module, I managed the authentication part, however as far as I understood, flickr has no profile url for the 'me' api, we should issue a call to http://api.flickr.com/services/rest/?method=flickr.people.getInfo&user_id=XXX where XXX is the user id of the logged user. In order to get the user_id it's necessary a previous call to method=flickr.people.findByUsername&username=YYY where YYY is the user name that can be grabbed from the authorization process:

I don't need the flickr access, I'm doing this just for fun so my knowledge of flikr is very poor, I didn't even have an account before and I'm wondering if all this process is correct and worths this effort, so any help will be appreciated.

MrSwitch commented 11 years ago

Hey,

Instead of defining the path as a string, give it a function, i.e. checkout modules/soundcloud.js

The first parameter is the options which initially calls hello.api(), the second it an asynchronous callback handler

e.g. For flickr you might want to define the "me" path as...

        'me' : function(p, callback){
            hello.api("flickr:/path/to/getcurrentuser", function(response){
                callback( "/api/method=flickr.people.findByUsername&username="+response.user_id +"/");
            });
        }

Make sense?, albeit Its pretty horrible, i did this for yahoo before i discovered their SQL like REST alternative.

I hope that helps, Let us know how you get on. And yeah, do it for the challenge!

Pasvaz commented 11 years ago

Yes that's what I did, but I still have a problem, Flickr doesn't recognize the 'callback' name for the jsonp callback, we have to use "jsoncallback=__lx_jsonpXX" or flickr will add the jsonFlickrApi() callback by default, so right now I'm trying to modify the _jsonp() and related functions in order to accept custom callback name, atm it sorta works but it's very hackish and I'm trying to find a more elegant solution without refactoring the whole core. An idea could be to add the callback element to the provider class, so before to build the url to that will be sent to the oauth proxy it can check if exists an element callback inside the provider and use that setting.

MrSwitch commented 11 years ago

Hey, yeah thats definitely a problem.

I do have a fix in mind,

in hello.api there is

// Otherwise we're on to the old school, IFRAME hacks and JSONP // Preprocess the parameters // Change the p parameters if("jsonp" in o){ o.jsonp(p,qs); }

In the module/flickr.js, there can be defined a JSONP function which modifies the parameters passed to it, variable "qs" in the above code.

hello.init('flickr', { ... jsonp:function(p,qs){ if(p.method==="GET"){ delete qs.callback; qs.callbackJSON = '?'; } } ... });

Then in hello.utils.jsonp make the field pattern match an attribute with '?'

Thats the basics, the definition of qs.callback = '?' actually comes later. If you are already fixing it then i'll critique your changes, otherwise i'll work on fixing this soon.

A

Andrew Dodson | Freelance Web Developer | +61 (0) 452 411 861 | http://adodson.com

On 25 June 2013 16:44, Pasquale Vazzana notifications@github.com wrote:

Yes that's what I did, but I still have a problem, Flickr doesn't recognize the 'callback' name for the jsonp callback, we have to use "jsoncallback=__lx_jsonpXX" or flickr will add the jsonFlickrApi() callback by default, so right now I'm trying to modify the _jsonp() and related functions in order to accept custom callback name, atm it sorta works but it's very hackish and I'm trying to find a more elegant solution without refactoring the whole core. An idea could be to add the callback element to the provider class, so before to build the url to that will be sent to the oauth proxy it can check if exists an element callback inside the provider and use that setting.

— Reply to this email directly or view it on GitHubhttps://github.com/MrSwitch/hello.js/issues/5#issuecomment-19956216 .

Pasvaz commented 11 years ago

Ok I changed my code and the new modifies follow your guidelines, I made some minor changes to the core and now it works flawlessly. Doing delete qs.callback inside the provider.jsonp is useless because hello.js calls the provider.jsonp as preprocessor so it can't deal with the qs.callback : hello will add qs.callback = '?' after the o.jsonp(), it would be better to use jsonp as post processor. Despite that, my solution works quite well, there is still a callback in excess in the url ( &jsoncallback=__lx_jsonp0&callback%3D__lx_jsonp0&) but it's ok, it will never be fired, it's just verbose and not elegant, but harmless. Considering that, afaik, o.jsonp is not used by any other provider, if you have no reason to use o.jsonp before to add the callback, I would move the check if("jsonp" in o){ as postprocessor so I can fix this excessive verbosity, otherwise it's fine in this way. I'll add some api functions, like albums and photos and I'll submit the code.

MrSwitch commented 11 years ago

Sounds great, thanks alot.

On 26 June 2013 10:18, Pasquale Vazzana notifications@github.com wrote:

Ok I changed my code and the new modifies follow your guidelines, I made some minor changes to the core and now it works flawlessly. Doing delete qs.callback inside the provider.jsonp is useless because hello.js calls the provider.jsonp as preprocessor so it can't deal with the qs.callback: hello will add qs.callback = '?' after the o.jsonp(), it would be better to use jsonp as post processor. Despite that, my solution works quite well, there is still a callback in excess in the url ( &jsoncallback=__lx_jsonp0&callback%3D__lx_jsonp0&) but it's ok, it will never be fired, it's just verbose and not elegant, but harmless, if you have no reason to use o.jsonp before to add the callback, I would move the check if("jsonp" in o){ as postprocessor so I can fix this excessive verbosity, otherwise it's fine in this way. I'll add some api functions, like albums and photos and I'll submit the code.

— Reply to this email directly or view it on GitHubhttps://github.com/MrSwitch/hello.js/issues/5#issuecomment-20018591 .

MrSwitch commented 11 years ago

Thanks Pasvaz, your flickr module is now in the main repo and i've also added it to the API explorer at http://adodson.com/hello.js/tests/

If you have any more issues please put them up, its great to get feedback from a competent dev such as yourself.

Pasvaz commented 11 years ago

I gave just a small contribute, thank you for this great sdk Andrew and keep up the good work.