Unable to create Custom Repository Role without permissions

[andriyun]

In the current implementation, the client allows you to request a Custom Repository Role for an Organization without specifying the required keys.
CreateCustomRepoRole use as argument opts *CreateOrUpdateCustomRoleOptions which has all key as optional https://github.com/google/go-github/blob/master/github/orgs_custom_roles.go#L60

As a result, the client will allow the creation request to GitHub and will fail with an error.

In most cases, what is missing might be clear and reasonable.

But it doesn't work when creating a custom role with no permissions. In other words when CreateCustomRepoRole gets an empty array as a Permission key.

Due to omitempty in JSON tag of the Permission key, an empty array as value is omitted from the final request to GitHub API, while it is required by the API

In our case, we are using custom roles that might have empty permissions set for flexibility's sake to extend them in the future

[gmlewis]

It sounds like we need a new endpoint with an additional private struct which removes the omitempty from the existing CreateOrUpdateCustomRoleOptions struct, but is otherwise identical. We've done things like this in other places.
For example, here:

go-github/github/orgs_properties.go

Lines 112 to 116 in 474a48f

	params := struct {
	Properties []*CustomProperty `json:"properties"`
	}{
	Properties: properties,
	}

@andriyun - would you like to make a PR to address this, or shall I open this up to other contributors to this repo?

[andriyun]

I'll be glad to submit PR with changes to fix it :)

[gmlewis]

Thank you, @andriyun ! It's yours.

[andriyun]

Hey @gmlewis
Sorry for late question. I didn't get your idea about creating new endpoint. I could neither find other cases you are referring too.
It's true at there are some similar constructions there with CreateOrUpdate****Options
However there are not used separate Create Update` methods.

I've added PR with changes #3235
Please take a look

[gmlewis]

So I was thinking that this could be achieved without breaking the current API by adding separate endpoint(s) that specifically handle the NoPermissions case, and internally uses modified structs where the omitempty has been removed from the permissions field.

Does that make sense?

I've looked at #3235 but was hoping to not break the current API, which that PR does.

[andriyun]

I understand your point now. It makes perfect sense.

I changed the PR in such a way, that it doesn't break the current API.
However, I couldn't run tests locally without fails. The fails I see look not relevant for changes I have added