Guard against faulty affiliations GraphQL filter

mislav · mislav · commit 5726376ab18f · 2020-03-25T08:26:06.000+01:00
This API was just fixed in github.com, but it will take a while for the
change to propagate to GitHub Enterprise installs, so guard ourselves
from false positives when querying forks.
diff --git a/api/queries_repo.go b/api/queries_repo.go
@@ -245,6 +245,7 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) {
 					name
 					owner { login }
 					url
+					viewerPermission
 				}
 			}
 		}
@@ -253,8 +254,12 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) {
 		return nil, err
 	}
 
-	if len(result.Repository.Forks.Nodes) > 0 {
-		return &result.Repository.Forks.Nodes[0], nil
+	forks := result.Repository.Forks.Nodes
+	// we check ViewerCanPush, even though we expect it to always be true per
+	// `affiliations` condition, to guard against versions of GitHub with a
+	// faulty `affiliations` implementation
+	if len(forks) > 0 && forks[0].ViewerCanPush() {
+		return &forks[0], nil
 	}
 	return nil, &NotFoundError{errors.New("no fork found")}
 }
diff --git a/command/pr_create_test.go b/command/pr_create_test.go
@@ -96,6 +96,10 @@ func TestPRCreate_alreadyExistsDifferentBase(t *testing.T) {
 	initBlankContext("OWNER/REPO", "feature")
 	http := initFakeHTTP()
 	http.StubRepoResponse("OWNER", "REPO")
+	http.StubResponse(200, bytes.NewBufferString(`
+	{ "data": { "repository": { "forks": { "nodes": [
+	] } } } }
+	`))
 	http.StubResponse(200, bytes.NewBufferString(`
 		{ "data": { "repository": { "pullRequests": { "nodes": [
 			{ "url": "https://github.com/OWNER/REPO/pull/123",
diff --git a/context/remote_test.go b/context/remote_test.go
@@ -137,7 +137,8 @@ func Test_resolvedRemotes_forkLookup(t *testing.T) {
 		{ "id": "FORKID",
 		  "url": "https://github.com/FORKOWNER/REPO",
 		  "name": "REPO",
-		  "owner": { "login": "FORKOWNER" }
+		  "owner": { "login": "FORKOWNER" },
+		  "viewerPermission": "WRITE"
 		}
 	] } } } }
 	`))

Original file line number	Diff line number	Diff line change
`@@ -245,6 +245,7 @@ func RepoFindFork(client Client, repo ghrepo.Interface) (Repository, error) {`
`245`	`245`	`name`
`246`	`246`	`owner { login }`
`247`	`247`	`url`
	`248`	`+ viewerPermission`
`248`	`249`	`}`
`249`	`250`	`}`
`250`	`251`	`}`
`@@ -253,8 +254,12 @@ func RepoFindFork(client Client, repo ghrepo.Interface) (Repository, error) {`
`253`	`254`	`return nil, err`
`254`	`255`	`}`
`255`	`256`
`256`		`- if len(result.Repository.Forks.Nodes) > 0 {`
`257`		`- return &result.Repository.Forks.Nodes[0], nil`
	`257`	`+ forks := result.Repository.Forks.Nodes`
	`258`	`+ // we check ViewerCanPush, even though we expect it to always be true per`
	`259`	+ // `affiliations` condition, to guard against versions of GitHub with a
	`260`	+ // faulty `affiliations` implementation
	`261`	`+ if len(forks) > 0 && forks[0].ViewerCanPush() {`
	`262`	`+ return &forks[0], nil`
`258`	`263`	`}`
`259`	`264`	`return nil, &NotFoundError{errors.New("no fork found")}`
`260`	`265`	`}`