Fix problem with pr #303, adding "maintain" and "triage" for repo collaborators

[romaninsh]

Due to type validation in previous PR #303 did not enable new roles for collaborators (only for teams) as per https://developer.github.com/v3/repos/collaborators/#add-user-as-a-collaborator collaborators can also be "maintain" and "triage":

Error: unexpected permission value: maintain
Error: unexpected permission value: maintain

This PR updates role validation and also updates documentation for repository_collaborator.

More info: https://github.com/terraform-providers/terraform-provider-github/pull/303#issuecomment-610409485

(my first contribution!)

[romaninsh]

@paultyng @liamg - my first PR here on some work you did previously. I hope it's OK!

[anGie44]

@romaninsh @jcudit updating the respository_collaborator_test with a test case that checks for these new permission types (maintain and/or triage) could be helpful as well :) looks like it's hardcoded to only check the admin permission

[romaninsh]

@anGie44 - I tried to find a way to update the test, but couldn't figure it out. Could you give some example how to do it?

[anGie44]

hi @romaninsh, sure thing! (@jcudit can you weigh in here as well for best practice when updating test cases?) so in resource_github_repository_collaborator_test.go, the basic test for example, refers to the expectedPermission const defined on line 17. i'm thinking one way to test for the different permissions could be to instead add steps like below and define permissions within the method like:

adminPermission := "admin"
maintainPermission := "maintain"
resource.ParallelTest(t, resource.TestCase{
		PreCheck:     func() { testAccPreCheck(t) },
		Providers:    testAccProviders,
		CheckDestroy: testAccCheckGithubRepositoryCollaboratorDestroy,
		Steps: []resource.TestStep{
			{
				Config: testAccGithubRepositoryCollaboratorConfig(repoName, testCollaborator, adminPermission),
			},
			{
				ResourceName:      rn,
				ImportState:       true,
				ImportStateVerify: true,
			},
			{
				Config: testAccGithubRepositoryCollaboratorConfig(repoName, testCollaborator, maintainPermission),
			},
			{
				ResourceName:      rn,
				ImportState:       true,
				ImportStateVerify: true,
			},
		},
	})

*This of course has the side-effect that the method testAccGithubRepositoryCollaboratorConfig needs to handle an additional string parameter for each permission type you pass in.
Hope this helps :) Let me know if you have any follow-up questions!

[jcudit]

The boilerplate referenced above is a good starting point. For more context, the function changing, getRepoPermission, is used by two resources and one data source. Only one use case currently has tests.

Take a look at this similar test logic to get an idea of how this was approached previously.

[anGie44]

hi @romaninsh, thanks again for this contribution and bug fix! closing this PR as the continuation of the implementation now lives in #457. please reach out with any questions or concerns if they arise :)