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

fitnessActivities requires an id, and other API thoughts #2

Open
RandomEtc opened this issue Mar 4, 2012 · 3 comments
Open

fitnessActivities requires an id, and other API thoughts #2

RandomEtc opened this issue Mar 4, 2012 · 3 comments

Comments

@RandomEtc
Copy link

Thanks for the library, and the recent changes to make it so that the access_token is easier to set per request.

After the recent changes to API permissions (so that developers can now access their own data), I started a new app that requests runkeeper.fitnessActivityFeed(access_token,callback,function(data){...}) and then loops over the returned data.items and loads that item's uri. At the moment I can't use your runkeeper.fitnessActivities(...) method because each item provides its own uri - I assume that method is just a stub and you haven't got to it yet?

Given the fact that RunKeeper's API provides uris dynamically for each resource, it might be more appropriate to provide a general purpose get method that accepts a (relative) uri, a media_type and an access_token. Something like:

HealthGraph.prototype.get = function(uri, media_type, access_token, callback) {

    console.log("This method is -- " + uri);

    var request_details = {
        method: 'GET',
        headers: {
            'Accept': media_type,
            'Authorization': 'Bearer ' + access_token
        },
        uri: "https://" + this.api_domain + uri
    };

    console.log(request_details);

    request(request_details, function(error, response, body) {
        console.log(request_details.method + " " + request_details.uri);
        console.log("ERROR");
        console.log(error);
        console.log("RESPONSE");
        console.log(response);
        console.log("BODY");
        console.log(body);
        if (error) {
            callback(error);
        }
        else {
            callback(null, body);
        }
    });
};

For me, this method can be used to call /fitnessActivities (the uri given by /users for accessing fitness_activities) with media_type application/vnd.com.runkeeper.FitnessActivityFeed+json and then loop over each item and call the method again with the given uri and media_type as application/vnd.com.runkeeper.FitnessActivity+json.

...

To initialize the API, instead of loading your api.json file you'd do a call to /user with media_type application/vnd.com.runkeeper.User+json and then discover all the other endpoints. The docs indicate that the authors intend the /user endpoint to be the only one hard-coded in your application. It's a little strange to me to "discover" endpoints but hard-code media types, but I think that's OK :)

Note I've also modified callback to receive an error as the first argument, as is common in node.js libraries of this kind.

Anyway, let me know what you think. I'm happy to implement this and push it up to my fork if you'd like to take a look before committing to it.

@marksoper
Copy link
Owner

Tom, I sent you an email in response by replying to the github email (appended below). Not sure if that got through. In any case, everything you're suggesting sounds good. Please submit a pull request and I'll approve it.

Mark

Tom, this sounds awesome! This was some early node work for me, so your 2nd set of eyes and these improvements are really valuable.

Perhaps the easiest way would be for you to submit a pull request when you're ready and I'll do it.

Mark

@RandomEtc
Copy link
Author

Great, thanks Mark. It's a weekend project for me but hopefully I'll find a moment soon.

@RandomEtc
Copy link
Author

I haven't yet found a moment to get these changes upstream but I pushed my own usage of the RunKeeper API in node up to https://github.com/RandomEtc/runboxer - hopefully I'll get a chance to merge these approaches and get the changes back here soon.

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

No branches or pull requests

2 participants