Go-github: Return app_id for an Installation

[sridharavinash]

I noticed that get single installation endpoint query here https://github.com/google/go-github/blob/master/github/apps.go#L100 returns an Installation but without an app_id. The app_id is returned in the call to https://developer.github.com/v3/apps/#get-a-single-installation.

I'd like to propose adding an AppID to the Installation struct here https://github.com/google/go-github/blob/master/github/apps_installation.go#L14 .
I also plan on updating the https://github.com/google/go-github/blob/master/github/apps_test.go#L84 test to include an app_id to test.

I have a PR ready to go with the addition of the field to the struct. Is that something that would be considered for merging?

🙇 Thanks for reading.

[kshitij10496]

Thanks for raising this issue.
After some digging, I found that the Installation struct could use some new fields for wrapping a response.

Reference: Get a Single Installation

{
  "id": 1,
  "account": {
    "login": "github",
    "id": 1,
    "url": "https://api.github.com/orgs/github",
    "repos_url": "https://api.github.com/orgs/github/repos",
    "events_url": "https://api.github.com/orgs/github/events",
    "hooks_url": "https://api.github.com/orgs/github/hooks",
    "issues_url": "https://api.github.com/orgs/github/issues",
    "members_url": "https://api.github.com/orgs/github/members{/member}",
    "public_members_url": "https://api.github.com/orgs/github/public_members{/member}",
    "avatar_url": "https://github.com/images/error/octocat_happy.gif",
    "description": "A great organization"
  },
  "access_tokens_url": "https://api.github.com/installations/1/access_tokens",
  "repositories_url": "https://api.github.com/installation/repositories",
  "html_url": "https://github.com/organizations/github/settings/installations/1",
  "app_id": 1,
  "target_id": 1,
  "target_type": "Organization",
  "permissions": {
    "metadata": "read",
    "contents": "read",
    "issues": "write",
    "single_file": "write"
  },
  "events": [
    "push",
    "pull_request"
  ],
  "single_file_name": "config.yml",
  "repository_selection": "selected"
}

Currently, we support only few fields of the response in the _Installation_ struct.

https://github.com/google/go-github/blob/ddf9d548a0f4d9b3fd42dd2add86e2c804860fef/github/apps_installation.go#L14-L20

I think in order to completely address this issue, we should also add the remaining fields.
@sridharavinash It would ideal if you can add them as well in #852 . If not, I can work on addressing it.
@gmlewis What do you think about this?

[gmlewis]

I think that if we are consistent with the rest of the repo in selectively choosing which fields to add, then that would be great.

For example, in the majority of structs, we don't enumerate every possible *_url field simply because they aren't useful when using our Go package for all API access. In other words, the url fields don't typically provide benefit to the user of our package.

But the non-url fields we typically try to support, so I'm in favor of adding those.

[sridharavinash]

@sridharavinash It would ideal if you can add them as well in #852 . If not, I can work on addressing it.

@kshitij10496 @gmlewis I'm happy to add the extra non-url fields where it makes sense. I'm inclined to address those in a separate PR, but can be convinced to include it in the current one 😄.

From a user's standpoint I required the app_id to make associations with the installation_id since these can be different. The other interesting fields are the permissions and events so that a user could potentially filter apps that match a criteria based on app permissions or events subscribed.

[gmlewis]

Sure, a separate PR would be fine.

[sridharavinash]

Awesome! Thanks for merging! I'll create a new PR to address the remaining fields. 🙇