Repository: google/go-github
Pull request #2611 (2022-12-29 13:41:02)

Add support for GitHub Environments for Pro/Teams pricing plans
The PR fixes the issue reported on https://github.com/google/go-github/issues/2602

In June GitHub made the environments and environment protection rules [generally available](https://github.blog/changelog/2021-06-22-github-actions-environments-environment-protection-rules-and-environment-secrets-are-generally-available/), however, the wait_timer and reviewers remain limited to enterprise.

Code was written to send `wait_timer` and `reviewers` with default values if no value is present, so existing settings can be cleared. 
Due to this, the code was returning 422 when called to create environments for Pro/Team private repos, as these parameters are only supported for enterprise or public repos.

PR intends to handle the error with a retry when the following conditions are met:
- error code is 422
- wait_timer is the default value (0)
- reviewers is the default value ([])

When retrying the call, a new object is sent, which only contains the parameters that are accepted for Pro/Teams private repos.

AnitaErnszt: @gmlewis Please kindly check if this implementation makes sense. I have tested the code locally for:
- pro/teams private repos -> successfully creates the environment
- free plan public repos -> successfully creates the environment
- free plan private repos -> 404, which is expected

private enterprise repos should have the same 'route' as the free plan public repos.

Unfortunately, I don't know how to extend the testing function to test the different 'paths'
codecov[bot]: # [Codecov](https://codecov.io/gh/google/go-github/pull/2611?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) Report
> Merging [#2611](https://codecov.io/gh/google/go-github/pull/2611?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) (0246f7d) into [master](https://codecov.io/gh/google/go-github/commit/c5583e771c6cf97aa86a86b9164129f59b949363?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) (c5583e7) will **decrease** coverage by `0.11%`.
> The diff coverage is `43.47%`.

```diff
@@            Coverage Diff             @@
##           master    #2611      +/-   ##
==========================================
- Coverage   97.99%   97.88%   -0.12%     
==========================================
  Files         127      127              
  Lines       10948    10969      +21     
==========================================
+ Hits        10729    10737       +8     
- Misses        150      162      +12     
- Partials       69       70       +1     
```


| [Impacted Files](https://codecov.io/gh/google/go-github/pull/2611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) | Coverage Δ | |
|---|---|---|
| [github/repos\_environments.go](https://codecov.io/gh/google/go-github/pull/2611/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google#diff-Z2l0aHViL3JlcG9zX2Vudmlyb25tZW50cy5nbw==) | `87.85% <43.47%> (-12.15%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google)

AnitaErnszt: @gmlewis I have added the tests. 
I wasn't sure how to decide what API response to return (422 or 200), so I used a counter for now. I'm open to suggestions
AnitaErnszt: On another note the contributor guide is recommending to run `golint` however, this project has been [deprecated](https://github.com/golang/lint) 
gmlewis: Thank you, @Xorima !
Merging.

Patch:
diff --git a/github/repos_environments.go b/github/repos_environments.go
index 365f8d92..2e85fdf9 100644
--- a/github/repos_environments.go
+++ b/github/repos_environments.go
@@ -9,6 +9,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/http"
 )
 
 // Environment represents a single environment in a repository.
@@ -168,6 +169,13 @@ type CreateUpdateEnvironment struct {
 	DeploymentBranchPolicy *BranchPolicy   `json:"deployment_branch_policy"`
 }
 
+// createUpdateEnvironmentNoEnterprise represents the fields accepted for Pro/Teams private repos.
+// Ref: https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment
+// See https://github.com/google/go-github/issues/2602 for more information.
+type createUpdateEnvironmentNoEnterprise struct {
+	DeploymentBranchPolicy *BranchPolicy `json:"deployment_branch_policy"`
+}
+
 // CreateUpdateEnvironment create or update a new environment for a repository.
 //
 // GitHub API docs: https://docs.github.com/en/rest/deployments/environments#create-or-update-an-environment
@@ -179,6 +187,33 @@ func (s *RepositoriesService) CreateUpdateEnvironment(ctx context.Context, owner
 		return nil, nil, err
 	}
 
+	e := new(Environment)
+	resp, err := s.client.Do(ctx, req, e)
+	if err != nil {
+		// The API returns 422 when the pricing plan doesn't support all the fields sent.
+		// This path will be executed for Pro/Teams private repos.
+		// For public repos, regardless of the pricing plan, all fields supported.
+		// For Free plan private repos the returned error code is 404.
+		// We are checking that the user didn't try to send a value for unsupported fields,
+		// and return an error if they did.
+		if resp != nil && resp.StatusCode == http.StatusUnprocessableEntity && environment != nil && len(environment.Reviewers) == 0 && environment.GetWaitTimer() == 0 {
+			return s.createNewEnvNoEnterprise(ctx, u, environment)
+		}
+		return nil, resp, err
+	}
+	return e, resp, nil
+}
+
+// createNewEnvNoEnterprise is an internal function for cases where the original call returned 422.
+// Currently only the `deployment_branch_policy` parameter is supported for Pro/Team private repos.
+func (s *RepositoriesService) createNewEnvNoEnterprise(ctx context.Context, u string, environment *CreateUpdateEnvironment) (*Environment, *Response, error) {
+	req, err := s.client.NewRequest("PUT", u, &createUpdateEnvironmentNoEnterprise{
+		DeploymentBranchPolicy: environment.DeploymentBranchPolicy,
+	})
+	if err != nil {
+		return nil, nil, err
+	}
+
 	e := new(Environment)
 	resp, err := s.client.Do(ctx, req, e)
 	if err != nil {


Test patch:
diff --git a/github/repos_environments_test.go b/github/repos_environments_test.go
index 93d0fc25..0b27da93 100644
--- a/github/repos_environments_test.go
+++ b/github/repos_environments_test.go
@@ -220,6 +220,110 @@ func TestRepositoriesService_CreateEnvironment(t *testing.T) {
 	})
 }
 
+func TestRepositoriesService_CreateEnvironment_noEnterprise(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	input := &CreateUpdateEnvironment{}
+	callCount := 0
+
+	mux.HandleFunc("/repos/o/r/environments/e", func(w http.ResponseWriter, r *http.Request) {
+		v := new(CreateUpdateEnvironment)
+		json.NewDecoder(r.Body).Decode(v)
+
+		testMethod(t, r, "PUT")
+		if callCount == 0 {
+			w.WriteHeader(http.StatusUnprocessableEntity)
+			callCount++
+		} else {
+			want := &CreateUpdateEnvironment{}
+			if !cmp.Equal(v, want) {
+				t.Errorf("Request body = %+v, want %+v", v, want)
+			}
+			fmt.Fprint(w, `{"id": 1, "name": "staging",	"protection_rules": []}`)
+		}
+	})
+
+	ctx := context.Background()
+	release, _, err := client.Repositories.CreateUpdateEnvironment(ctx, "o", "r", "e", input)
+	if err != nil {
+		t.Errorf("Repositories.CreateUpdateEnvironment returned error: %v", err)
+	}
+
+	want := &Environment{ID: Int64(1), Name: String("staging"), ProtectionRules: []*ProtectionRule{}}
+	if !cmp.Equal(release, want) {
+		t.Errorf("Repositories.CreateUpdateEnvironment returned %+v, want %+v", release, want)
+	}
+}
+
+func TestRepositoriesService_createNewEnvNoEnterprise(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	input := &CreateUpdateEnvironment{
+		DeploymentBranchPolicy: &BranchPolicy{
+			ProtectedBranches:    Bool(true),
+			CustomBranchPolicies: Bool(false),
+		},
+	}
+
+	mux.HandleFunc("/repos/o/r/environments/e", func(w http.ResponseWriter, r *http.Request) {
+		v := new(createUpdateEnvironmentNoEnterprise)
+		json.NewDecoder(r.Body).Decode(v)
+
+		testMethod(t, r, "PUT")
+		want := &createUpdateEnvironmentNoEnterprise{
+			DeploymentBranchPolicy: &BranchPolicy{
+				ProtectedBranches:    Bool(true),
+				CustomBranchPolicies: Bool(false),
+			},
+		}
+		if !cmp.Equal(v, want) {
+			t.Errorf("Request body = %+v, want %+v", v, want)
+		}
+		fmt.Fprint(w, `{"id": 1, "name": "staging",	"protection_rules": [{"id": 1, "node_id": "id", "type": "branch_policy"}], "deployment_branch_policy": {"protected_branches": true, "custom_branch_policies": false}}`)
+	})
+
+	ctx := context.Background()
+	release, _, err := client.Repositories.createNewEnvNoEnterprise(ctx, "repos/o/r/environments/e", input)
+	if err != nil {
+		t.Errorf("Repositories.createNewEnvNoEnterprise returned error: %v", err)
+	}
+
+	want := &Environment{
+		ID:   Int64(1),
+		Name: String("staging"),
+		ProtectionRules: []*ProtectionRule{
+			{
+				ID:     Int64(1),
+				NodeID: String("id"),
+				Type:   String("branch_policy"),
+			},
+		},
+		DeploymentBranchPolicy: &BranchPolicy{
+			ProtectedBranches:    Bool(true),
+			CustomBranchPolicies: Bool(false),
+		},
+	}
+	if !cmp.Equal(release, want) {
+		t.Errorf("Repositories.createNewEnvNoEnterprise returned %+v, want %+v", release, want)
+	}
+
+	const methodName = "createNewEnvNoEnterprise"
+	testBadOptions(t, methodName, func() (err error) {
+		_, _, err = client.Repositories.createNewEnvNoEnterprise(ctx, "\n", input)
+		return err
+	})
+
+	testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
+		got, resp, err := client.Repositories.createNewEnvNoEnterprise(ctx, "repos/o/r/environments/e", input)
+		if got != nil {
+			t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
+		}
+		return resp, err
+	})
+}
+
 func TestRepositoriesService_DeleteEnvironment(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()