Support fetching resources by id

[joshwlewis]

Currently, this addon doesn't accommodate fetching github-user or github-repository by it's id. That's because GitHub has an unusual API that requires using a different URL depending on whether you are fetching by name or id.

For example, this fails:

GET https://api.github.com/users/917672

{
  "message": "Not Found",
  "documentation_url": "https://developer.github.com/v3/users/#get-a-single-user"
}

But this works:

GET https://api.github.com/user/917672

{
  "login": "joshwlewis",
  "id": 917672,
  "avatar_url": "https://avatars1.githubusercontent.com/u/917672?v=4",
  "gravatar_id": "",
  "url": "https://api.github.com/users/joshwlewis",
  "html_url": "https://github.com/joshwlewis",
  "followers_url": "https://api.github.com/users/joshwlewis/followers",
  "following_url": "https://api.github.com/users/joshwlewis/following{/other_user}",
  "gists_url": "https://api.github.com/users/joshwlewis/gists{/gist_id}",
  "starred_url": "https://api.github.com/users/joshwlewis/starred{/owner}{/repo}",
  "subscriptions_url": "https://api.github.com/users/joshwlewis/subscriptions",
  "organizations_url": "https://api.github.com/users/joshwlewis/orgs",
  "repos_url": "https://api.github.com/users/joshwlewis/repos",
  "events_url": "https://api.github.com/users/joshwlewis/events{/privacy}",
  "received_events_url": "https://api.github.com/users/joshwlewis/received_events",
  "type": "User",
  "site_admin": false,
  "name": "Josh W Lewis",
  "company": "Heroku",
  "blog": "http://joshwlewis.com",
  "location": "Memphis, TN",
  "email": null,
  "hireable": null,
  "bio": null,
  "public_repos": 68,
  "public_gists": 7,
  "followers": 57,
  "following": 51,
  "created_at": "2011-07-15T13:16:41Z",
  "updated_at": "2017-10-02T16:34:22Z"
}

This PR adds support for fetching by id, by detecting if the provided id is an integer.

It is possible to name both resources with digits on GitHub, so this means there is a potential conflict if you want to find the user with the login "1234", but instead get the user with id 1234. This PR means we would prefer the id version, and I believe that's consistent with the way EmberData is designed (heavily based on remote ids).

[elwayman02]

This looks great! @Dhaulagiri @srvance do you have any thoughts on this approach? I want to make sure there aren't any real-world use-cases we're overlooking where this is problematic.

[Dhaulagiri]

I know @joshwlewis is making these changes for real use cases in our app so I defer to his judgement as he has a lot more experience with github's api than I do.

@joshwlewis would you mind updating the readme with examples of these calls as well?

[joshwlewis]

@Dhaulagiri the readme has been updated.

@elwayman02 the only caveat I can think of I already listed on the PR description. However, I feel pretty strongly that preferring fetching by id is correct. In my experience, it's fairly common for repository names and user logins to change, but GitHub doesn't give notification when this happens. As a result, fetching resources by a name is considerably less stable than fetching by id.

[elwayman02]

I only have one question, otherwise this looks good. What happens if the ID passed is a string type? Will your code still read it as an ID instead of a name? I can see situations where some apps may be passing around strings because of how their backend stores the data, for example. It would be nice to have additional tests that verify the expected behavior, in particular so we don't accidentally break it in the future if someone decides to refactor your regexes for some reason.

[elwayman02]

nice!

[joshwlewis]

What happens if the ID passed is a string type? Will your code still read it as an ID instead of a name?

If it's a string of digits, this code will treat it as an id. =)

Tests added.

[elwayman02]

@Dhaulagiri do you mind handling this release to meet your app's timelines? Looks good to me once CI passes.

[Dhaulagiri]

Published in 0.5.0. Thanks @joshwlewis!

[srvance]

@elwayman02 Sorry for the slow response. This sounds like a good approach. My only suggestion on the implementation would be to encapsulate the integer check and use Number.isInteger() or parseInt() where supported.