Skip to content

fix repo create in org with license/ignore #3924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vilmibm merged 3 commits into from
Jun 30, 2021
Merged

fix repo create in org with license/ignore #3924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2723a01
fix repo generation in org with license/ignore
Jun 30, 2021
202168e
add nebula preview
Jun 30, 2021
589b695
test for org + license/ignore
Jun 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/cmd/factory/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg configGetter, appVersion string,
opts = append(opts,
api.AddHeaderFunc("Accept", func(req *http.Request) (string, error) {
accept := "application/vnd.github.merge-info-preview+json" // PullRequest.mergeStateStatus
accept += ", application/vnd.github.nebula-preview" // visibility when RESTing repos into an org
if ghinstance.IsEnterprise(getHost(req)) {
accept += ", application/vnd.github.antiope-preview" // Commit.statusCheckRollup
accept += ", application/vnd.github.shadow-cat-preview" // PullRequest.isDraft
Expand Down
10 changes: 5 additions & 5 deletions pkg/cmd/factory/http_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestNewHTTPClient(t *testing.T) {
wantHeader: map[string]string{
"authorization": "token MYTOKEN",
"user-agent": "GitHub CLI v1.2.3",
"accept": "application/vnd.github.merge-info-preview+json",
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
},
wantStderr: "",
},
Expand Down Expand Up @@ -69,7 +69,7 @@ func TestNewHTTPClient(t *testing.T) {
wantHeader: map[string]string{
"authorization": "",
"user-agent": "GitHub CLI v1.2.3",
"accept": "application/vnd.github.merge-info-preview+json",
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
},
wantStderr: "",
},
Expand All @@ -85,14 +85,14 @@ func TestNewHTTPClient(t *testing.T) {
wantHeader: map[string]string{
"authorization": "token MYTOKEN",
"user-agent": "GitHub CLI v1.2.3",
"accept": "application/vnd.github.merge-info-preview+json",
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
},
wantStderr: heredoc.Doc(`
* Request at <time>
* Request to http://<host>:<port>
> GET / HTTP/1.1
> Host: github.com
> Accept: application/vnd.github.merge-info-preview+json
> Accept: application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview
> Authorization: token ████████████████████
> User-Agent: GitHub CLI v1.2.3

Expand All @@ -113,7 +113,7 @@ func TestNewHTTPClient(t *testing.T) {
wantHeader: map[string]string{
"authorization": "token GHETOKEN",
"user-agent": "GitHub CLI v1.2.3",
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.antiope-preview, application/vnd.github.shadow-cat-preview",
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview, application/vnd.github.antiope-preview, application/vnd.github.shadow-cat-preview",
},
wantStderr: "",
},
Expand Down
97 changes: 93 additions & 4 deletions pkg/cmd/repo/create/create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,8 +622,8 @@ func TestRepoCreate_WithGitIgnore(t *testing.T) {
if repoName := reqBody.Name; repoName != "REPO" {
t.Errorf("expected %q, got %q", "REPO", repoName)
}
if repoVisibility := reqBody.Visibility; repoVisibility != "PRIVATE" {
t.Errorf("expected %q, got %q", "PRIVATE", repoVisibility)
if repoVisibility := reqBody.Visibility; repoVisibility != "private" {
t.Errorf("expected %q, got %q", "private", repoVisibility)
}
if ownerId := reqBody.OwnerId; ownerId != "OWNERID" {
t.Errorf("expected %q, got %q", "OWNERID", ownerId)
Expand Down Expand Up @@ -721,8 +721,97 @@ func TestRepoCreate_WithBothGitIgnoreLicense(t *testing.T) {
if repoName := reqBody.Name; repoName != "REPO" {
t.Errorf("expected %q, got %q", "REPO", repoName)
}
if repoVisibility := reqBody.Visibility; repoVisibility != "PRIVATE" {
t.Errorf("expected %q, got %q", "PRIVATE", repoVisibility)
if repoVisibility := reqBody.Visibility; repoVisibility != "private" {
t.Errorf("expected %q, got %q", "private", repoVisibility)
}
if ownerId := reqBody.OwnerId; ownerId != "OWNERID" {
t.Errorf("expected %q, got %q", "OWNERID", ownerId)
}
}

func TestRepoCreate_WithGitIgnore_Org(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)

cs.Register(`git remote add -f origin https://github\.com/OWNER/REPO\.git`, 0, "")
cs.Register(`git rev-parse --show-toplevel`, 0, "")

as, surveyTearDown := prompt.InitAskStubber()
defer surveyTearDown()

as.Stub([]*prompt.QuestionStub{
{
Name: "repoVisibility",
Value: "PRIVATE",
},
})

as.Stub([]*prompt.QuestionStub{
{
Name: "addGitIgnore",
Value: true,
},
})

as.Stub([]*prompt.QuestionStub{
{
Name: "chooseGitIgnore",
Value: "Go",
},
})

as.Stub([]*prompt.QuestionStub{
{
Name: "addLicense",
Value: false,
},
})

as.Stub([]*prompt.QuestionStub{
{
Name: "confirmSubmit",
Value: true,
},
})

reg := &httpmock.Registry{}
reg.Register(
httpmock.REST("GET", "users/OWNER"),
httpmock.StringResponse(`{ "node_id": "OWNERID", "type":"Organization" }`))
reg.Register(
httpmock.REST("GET", "gitignore/templates"),
httpmock.StringResponse(`["Actionscript","Android","AppceleratorTitanium","Autotools","Bancha","C","C++","Go"]`))
reg.Register(
httpmock.REST("POST", "orgs/OWNER/repos"),
httpmock.StringResponse(`{"name":"REPO", "owner":{"login": "OWNER"}, "html_url":"https://github.com/OWNER/REPO"}`))
httpClient := &http.Client{Transport: reg}

output, err := runCommand(httpClient, "OWNER/REPO", true)
if err != nil {
t.Errorf("error running command `repo create`: %v", err)
}

assert.Equal(t, "", output.String())
assert.Equal(t, "✓ Created repository OWNER/REPO on GitHub\n✓ Added remote https://github.com/OWNER/REPO.git\n", output.Stderr())

var reqBody struct {
Name string
Visibility string
OwnerId string
LicenseTemplate string
}

if len(reg.Requests) != 3 {
t.Fatalf("expected 3 HTTP request, got %d", len(reg.Requests))
}

bodyBytes, _ := ioutil.ReadAll(reg.Requests[2].Body)
_ = json.Unmarshal(bodyBytes, &reqBody)
if repoName := reqBody.Name; repoName != "REPO" {
t.Errorf("expected %q, got %q", "REPO", repoName)
}
if repoVisibility := reqBody.Visibility; repoVisibility != "private" {
t.Errorf("expected %q, got %q", "private", repoVisibility)
}
if ownerId := reqBody.OwnerId; ownerId != "OWNERID" {
t.Errorf("expected %q, got %q", "OWNERID", ownerId)
Expand Down
22 changes: 18 additions & 4 deletions pkg/cmd/repo/create/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"strings"

"github.com/cli/cli/api"
)
Expand Down Expand Up @@ -38,6 +39,9 @@ type repoTemplateInput struct {
func repoCreate(client *http.Client, hostname string, input repoCreateInput, templateRepositoryID string) (*api.Repository, error) {
apiClient := api.NewClientFromHTTP(client)

ownerName := input.OwnerID
isOrg := false

if input.TeamID != "" {
orgID, teamID, err := resolveOrganizationTeam(apiClient, hostname, input.OwnerID, input.TeamID)
if err != nil {
Expand All @@ -46,7 +50,9 @@ func repoCreate(client *http.Client, hostname string, input repoCreateInput, tem
input.TeamID = teamID
input.OwnerID = orgID
} else if input.OwnerID != "" {
orgID, err := resolveOrganization(apiClient, hostname, input.OwnerID)
var orgID string
var err error
orgID, isOrg, err = resolveOrganization(apiClient, hostname, input.OwnerID)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -109,12 +115,19 @@ func repoCreate(client *http.Client, hostname string, input repoCreateInput, tem
}

if input.GitIgnoreTemplate != "" || input.LicenseTemplate != "" {
input.Visibility = strings.ToLower(input.Visibility)
Copy link
Contributor

@samcoe samcoe Jun 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦

body := &bytes.Buffer{}
enc := json.NewEncoder(body)
if err := enc.Encode(input); err != nil {
return nil, err
}
repo, err := api.CreateRepoTransformToV4(apiClient, hostname, "POST", "user/repos", body)

path := "user/repos"
if isOrg {
path = fmt.Sprintf("orgs/%s/repos", ownerName)
Copy link
Contributor

@samcoe samcoe Jun 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty sure the answer is no, but can we just use the input.OwnerID here? Seems silly that the API couldn't resolve that.

Copy link
Contributor Author

@vilmibm vilmibm Jun 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, it's a graphql node ID.

Copy link
Contributor Author

@vilmibm vilmibm Jun 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but your confusion is warranted! it starts out life as an owner string and is then updated in place to be a node ID.

}

repo, err := api.CreateRepoTransformToV4(apiClient, hostname, "POST", path, body)
if err != nil {
return nil, err
}
Expand All @@ -141,12 +154,13 @@ func repoCreate(client *http.Client, hostname string, input repoCreateInput, tem
}

// using API v3 here because the equivalent in GraphQL needs `read:org` scope
func resolveOrganization(client *api.Client, hostname, orgName string) (string, error) {
func resolveOrganization(client *api.Client, hostname, orgName string) (string, bool, error) {
var response struct {
Copy link

@GypsyQueen99 GypsyQueen99 Jul 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

@GypsyQueen99 GypsyQueen99 Jul 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whooooo

NodeID string `json:"node_id"`
Type string
}
err := client.REST(hostname, "GET", fmt.Sprintf("users/%s", orgName), nil, &response)
return response.NodeID, err
return response.NodeID, response.Type == "Organization", err
}

// using API v3 here because the equivalent in GraphQL needs `read:org` scope
Expand Down