Teams.IsTeamRepoByID support for custom roles

[joshua-hancox]

Now that GitHub has created the custom roles feature, I think the implementation of Teams.IsTeamRepoByID (and BySlug) may need to be changed, or a new function added.

https://github.com/google/go-github/blob/master/github/teams.go#L411L427

Currently, we return a repository object, which contains information about which permissions the specified team has on the repository.

e.g.

    "permissions": {
        "admin": true,
        "maintain": true,
        "push": true,
        "triage": true,
        "pull": true
    }

However, the API that we interact with here also returns the name of the role that is assigned to that team on that repository.

e.g

    "role_name": "admin"

Previously this was not an issue, as it was possible to work out what the role name would be based on the permissions object. (since all role names are included there.) Now that custom roles are available, a role based on the admin role may be created, but with a custom name.

e.g.

    "permissions": {
        "admin": true,
        "maintain": true,
        "push": true,
        "triage": true,
        "pull": true
    }
    "role_name": "super-admin"

Based on what is returned from this function currently, we cannot get the name of the role, and the permissions object looks identical to the one returned when the built-in admin role is assigned.

Should we extend the Repository type to include the role_name? It seems a bit strange to me that the permissions object is included there since that describes an assignment between a team and a repo rather than just an attribute of the repo. But if we wanted to follow that pattern would the change to include role_name make sense?

[gmlewis]

Based on the API documentation you linked above, I don't see any other option but to include the role_name into the Repository struct, with a comment that it is returned only for certain API endpoints.

Although this does indeed seem strange, what alternatives can you think of?

[joshua-hancox]

I wondered if this function should really return a Repository struct at all? Since we're checking what permissions a specific team has on a specific repository, should we create/use a new struct like TeamRepositoryPermissions.

e.g.

type TeamRepositoryPermissions struct {
	Permissions         map[string]bool `json:"permissions,omitempty"`
	RoleName            *string `json:"role_name,omitempty"`
}

But understand that would be a breaking change so maybe not ideal.

[gmlewis]

Looking at the example response here: https://docs.github.com/en/rest/teams/teams#check-team-permissions-for-a-repository
I don't see how we cannot return a Repository struct.

For some reason, the GitHub v3 API team decided that all that information needed to be returned, so I don't think we want to discard the vast majority of it.

[joshua-hancox]

OK sure, understood. That being the case I guess we have no choice but to include role_name in the exiting repository struct?

I'd also like to include the below function in github-accessors.go:

// GetRoleName returns the role_name field if it's non-nil, zero value otherwise.
func (r *Repository) GetRoleName() string {
	if r == nil || r.RoleName == nil {
		return ""
	}
	return *r.RoleName
}

If you agree, I'll raise a PR sometime today.

[gmlewis]

Yes, go ahead with a PR but do not manually edit the accessors file as it is autogenerated. Please read CONTRIBUTING.md for details.

[joshua-hancox]

Ahh, I see. Thanks 😃 .

PR is here whenever you get a second.

#2379

[joshua-hancox]

Thankyou @gmlewis! Any idea when your next release will be?

[gmlewis]

Thankyou @gmlewis! Any idea when your next release will be?

Right now: https://github.com/google/go-github/releases/tag/v45.1.0
😄

[joshua-hancox]

Amazing! Thankyou