From 343896fdac9ee1aeee7152f66765832c31cc87f6 Mon Sep 17 00:00:00 2001
From: Alex Petrov <alex.petrov.vt@gmail.com>
Date: Thu, 6 Jul 2023 03:05:40 -0400
Subject: [PATCH] Do not interpret "branch" placeholder in `api` command when
 `GH_REPO` is set (#7626)

* fix(api): do not interpret "branch" placeholder when `GH_REPO` is set

Before, we would always interpret the "branch" placeholder, typically
setting it to the currently checked-out branch in the repository in the
current working directory. It didn't make sense to do that when the
`GH_REPO` environment variable was specified because the repository
would likely be different from the one in the current working directory.

Now, we instead report an error if both `GH_REPO` environment variable
and `branch` placeholder are specified.
---
 pkg/cmd/api/api.go      |  6 ++++++
 pkg/cmd/api/api_test.go | 26 ++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go
index 82eaecb8d..3ac83ad6b 100644
--- a/pkg/cmd/api/api.go
+++ b/pkg/cmd/api/api.go
@@ -3,6 +3,7 @@ package api
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -469,6 +470,11 @@ func fillPlaceholders(value string, opts *ApiOptions) (string, error) {
 				err = e
 			}
 		case "branch":
+			if os.Getenv("GH_REPO") != "" {
+				err = errors.New("unable to determine an appropriate value for the 'branch' placeholder")
+				return m
+			}
+
 			if branch, e := opts.Branch(); e == nil {
 				return branch
 			} else {
diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go
index bd2a94994..2d43daea7 100644
--- a/pkg/cmd/api/api_test.go
+++ b/pkg/cmd/api/api_test.go
@@ -1096,10 +1096,11 @@ func Test_fillPlaceholders(t *testing.T) {
 		opts  *ApiOptions
 	}
 	tests := []struct {
-		name    string
-		args    args
-		want    string
-		wantErr bool
+		name         string
+		args         args
+		repoOverride bool
+		want         string
+		wantErr      bool
 	}{
 		{
 			name: "no changes",
@@ -1236,9 +1237,26 @@ func Test_fillPlaceholders(t *testing.T) {
 			want:    "{}{ownership}/{repository}",
 			wantErr: false,
 		},
+		{
+			name:         "branch can't be filled when GH_REPO is set",
+			repoOverride: true,
+			args: args{
+				value: "repos/:owner/:repo/branches/:branch",
+				opts: &ApiOptions{
+					BaseRepo: func() (ghrepo.Interface, error) {
+						return ghrepo.New("hubot", "robot-uprising"), nil
+					},
+				},
+			},
+			want:    "repos/hubot/robot-uprising/branches/:branch",
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			if tt.repoOverride {
+				t.Setenv("GH_REPO", "hubot/robot-uprising")
+			}
 			got, err := fillPlaceholders(tt.args.value, tt.args.opts)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("fillPlaceholders() error = %v, wantErr %v", err, tt.wantErr)