From 015b9f7fea68cbbeb324556f2101d57f73a544db Mon Sep 17 00:00:00 2001 From: keijun-kumagai Date: Tue, 11 Jan 2022 01:49:19 +0900 Subject: [PATCH 1/8] fix(release): discussion category with assets --- pkg/cmd/release/create/create.go | 7 ++++++- pkg/cmd/release/create/http.go | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 1b834373f..bdfdf1460 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -418,7 +418,12 @@ func createRun(opts *CreateOptions) error { } if !opts.Draft { - rel, err := publishRelease(httpClient, newRelease.APIURL) + params := map[string]interface{}{} + params["draft"] = false + if opts.DiscussionCategory != "" { + params["discussion_category_name"] = opts.DiscussionCategory + } + rel, err := publishRelease(httpClient, newRelease.APIURL, params) if err != nil { return err } diff --git a/pkg/cmd/release/create/http.go b/pkg/cmd/release/create/http.go index fb082cd79..058b38c5b 100644 --- a/pkg/cmd/release/create/http.go +++ b/pkg/cmd/release/create/http.go @@ -134,8 +134,9 @@ func createRelease(httpClient *http.Client, repo ghrepo.Interface, params map[st return &newRelease, err } -func publishRelease(httpClient *http.Client, releaseURL string) (*shared.Release, error) { - req, err := http.NewRequest("PATCH", releaseURL, bytes.NewBufferString(`{"draft":false}`)) +func publishRelease(httpClient *http.Client, releaseURL string, params map[string]interface{}) (*shared.Release, error) { + bodyBytes, err := json.Marshal(params) + req, err := http.NewRequest("PATCH", releaseURL, bytes.NewBuffer(bodyBytes)) if err != nil { return nil, err } From 6bc0b693554f5ab6d3761f5ee5c46ed2d7df3815 Mon Sep 17 00:00:00 2001 From: keijun-kumagai Date: Thu, 13 Jan 2022 00:33:07 +0900 Subject: [PATCH 2/8] feat(http): supress linter --- pkg/cmd/release/create/http.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/release/create/http.go b/pkg/cmd/release/create/http.go index 058b38c5b..b5ee5c271 100644 --- a/pkg/cmd/release/create/http.go +++ b/pkg/cmd/release/create/http.go @@ -136,6 +136,9 @@ func createRelease(httpClient *http.Client, repo ghrepo.Interface, params map[st func publishRelease(httpClient *http.Client, releaseURL string, params map[string]interface{}) (*shared.Release, error) { bodyBytes, err := json.Marshal(params) + if err != nil { + return nil, err + } req, err := http.NewRequest("PATCH", releaseURL, bytes.NewBuffer(bodyBytes)) if err != nil { return nil, err From 32dada90bac83d0a4964a37704d02778124ff59c Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 14 Feb 2022 10:35:09 -0600 Subject: [PATCH 3/8] revert PR 4998 --- docs/install_linux.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/install_linux.md b/docs/install_linux.md index b45d7a599..b07e2478b 100644 --- a/docs/install_linux.md +++ b/docs/install_linux.md @@ -14,8 +14,8 @@ our release schedule. Install: ```bash -curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg | sudo dd of=/etc/apt/trusted.gpg.d/githubcli-archive-keyring.gpg -echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/trusted.gpg.d/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | sudo tee /etc/apt/sources.list.d/github-cli.list > /dev/null +curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg | sudo dd of=/usr/share/keyrings/githubcli-archive-keyring.gpg +echo "deb [arch=$(dpkg --print-architecture) signed-by=/usr/share/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | sudo tee /etc/apt/sources.list.d/github-cli.list > /dev/null sudo apt update sudo apt install gh ``` From 28d2b52769421e82b4cc3a46a619216ddd1a923e Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 15 Feb 2022 16:36:37 +0100 Subject: [PATCH 4/8] release create: warn about unpushed local git tag (#5104) If `gh release create ` was called and TAG exists locally but not on the remote, this warns about the unpushed tag to avoid recreating it on the remote. The user can pass a value for `--target` to silence the warning. --- api/queries_repo.go | 3 +- pkg/cmd/release/create/create.go | 27 ++++++++++- pkg/cmd/release/create/create_test.go | 65 ++++++++++++++++++++++++++- pkg/cmd/release/create/http.go | 21 +++++++++ 4 files changed, 112 insertions(+), 4 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index 65f1027a8..b7ea14915 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -5,13 +5,14 @@ import ( "context" "encoding/json" "fmt" - "github.com/cli/cli/v2/internal/ghinstance" "io" "net/http" "sort" "strings" "time" + "github.com/cli/cli/v2/internal/ghinstance" + "github.com/cli/cli/v2/internal/ghrepo" "github.com/shurcooL/githubv4" ) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 96f7245c1..4d0ea32c8 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -174,6 +174,7 @@ func createRun(opts *CreateOptions) error { return err } + var existingTag bool if opts.TagName == "" { tags, err := getTags(httpClient, baseRepo, 5) if err != nil { @@ -198,6 +199,7 @@ func createRun(opts *CreateOptions) error { return fmt.Errorf("could not prompt: %w", err) } if tag != createNewTagOption { + existingTag = true opts.TagName = tag } } @@ -213,6 +215,29 @@ func createRun(opts *CreateOptions) error { } } + var tagDescription string + if opts.RepoOverride == "" { + tagDescription, _ = gitTagInfo(opts.TagName) + // If there is a local tag with the same name as specified + // the user may not want to create a new tag on the remote + // as the local one might be annotated or signed. + // If the user specifies the target take that as explict instruction + // to create the tag on the remote pointing to the target regardless + // of local tag status. + // If a remote tag with the same name as specified exists already + // then a new tag will not be created so ignore local tag status. + if tagDescription != "" && !existingTag && opts.Target == "" { + remoteExists, err := remoteTagExists(httpClient, baseRepo, opts.TagName) + if err != nil { + return err + } + if !remoteExists { + return fmt.Errorf("tag %s exists locally but has not been pushed to %s, please push it before continuing or specify the `--target` flag to create a new tag", + opts.TagName, ghrepo.FullName(baseRepo)) + } + } + } + if !opts.BodyProvided && opts.IO.CanPrompt() { editorCommand, err := cmdutil.DetermineEditor(opts.Config) if err != nil { @@ -220,7 +245,6 @@ func createRun(opts *CreateOptions) error { } var generatedNotes *releaseNotes - var tagDescription string var generatedChangelog string params := map[string]interface{}{ @@ -236,7 +260,6 @@ func createRun(opts *CreateOptions) error { if opts.RepoOverride == "" { headRef := opts.TagName - tagDescription, _ = gitTagInfo(opts.TagName) if tagDescription == "" { if opts.Target != "" { // TODO: use the remote-tracking version of the branch ref diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index ea9c2aa44..1811c30d7 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -673,6 +673,8 @@ func Test_createRun_interactive(t *testing.T) { rs.Register(`git describe --tags --abbrev=0 v1\.2\.3\^`, 1, "") }, httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.GraphQL("RepositoryFindRef"), + httpmock.StringResponse(`{"data":{"repository":{"ref": {"id": "tag id"}}}}`)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases/generate-notes"), httpmock.StatusStringResponse(404, `{}`)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), @@ -690,6 +692,57 @@ func Test_createRun_interactive(t *testing.T) { }, wantOut: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", }, + { + name: "error when unpublished local tag and target not specified", + opts: &CreateOptions{ + TagName: "v1.2.3", + }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "tag exists") + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.GraphQL("RepositoryFindRef"), + httpmock.StringResponse(`{"data":{"repository":{"ref": {"id": ""}}}}`)) + }, + wantErr: "tag v1.2.3 exists locally but has not been pushed to OWNER/REPO, please push it before continuing or specify the `--target` flag to create a new tag", + }, + { + name: "create a release when unpublished local tag and target specified", + opts: &CreateOptions{ + TagName: "v1.2.3", + Target: "main", + }, + askStubs: func(as *prompt.AskStubber) { + as.StubPrompt("Title (optional)").AnswerWith("") + as.StubPrompt("Release notes"). + AssertOptions([]string{"Write my own", "Write using generated notes as template", "Write using git tag message as template", "Leave blank"}). + AnswerWith("Leave blank") + as.StubPrompt("Is this a prerelease?").AnswerWith(false) + as.StubPrompt("Submit?").AnswerWith("Publish release") + }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git tag --list`, 0, "tag exists") + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases/generate-notes"), + httpmock.StatusStringResponse(200, `{ + "name": "generated name", + "body": "generated body" + }`)) + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.StatusStringResponse(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`)) + }, + wantParams: map[string]interface{}{ + "draft": false, + "prerelease": false, + "tag_name": "v1.2.3", + "target_commitish": "main", + }, + wantOut: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", + }, } for _, tt := range tests { ios, _, stdout, stderr := iostreams.Test() @@ -739,7 +792,17 @@ func Test_createRun_interactive(t *testing.T) { } if tt.wantParams != nil { - bb, err := ioutil.ReadAll(reg.Requests[1].Body) + var r *http.Request + for _, req := range reg.Requests { + if req.URL.Path == "/repos/OWNER/REPO/releases" { + r = req + break + } + } + if r == nil { + t.Fatalf("no http requests for creating a release found") + } + bb, err := ioutil.ReadAll(r.Body) assert.NoError(t, err) var params map[string]interface{} err = json.Unmarshal(bb, ¶ms) diff --git a/pkg/cmd/release/create/http.go b/pkg/cmd/release/create/http.go index fb082cd79..07a37a4f6 100644 --- a/pkg/cmd/release/create/http.go +++ b/pkg/cmd/release/create/http.go @@ -2,6 +2,7 @@ package create import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -13,6 +14,7 @@ import ( "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/release/shared" + graphql "github.com/cli/shurcooL-graphql" ) type tag struct { @@ -26,6 +28,25 @@ type releaseNotes struct { var notImplementedError = errors.New("not implemented") +func remoteTagExists(httpClient *http.Client, repo ghrepo.Interface, tagName string) (bool, error) { + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), httpClient) + qualifiedTagName := fmt.Sprintf("refs/tags/%s", tagName) + var query struct { + Repository struct { + Ref struct { + ID string + } `graphql:"ref(qualifiedName: $tagName)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + variables := map[string]interface{}{ + "owner": graphql.String(repo.RepoOwner()), + "name": graphql.String(repo.RepoName()), + "tagName": graphql.String(qualifiedTagName), + } + err := gql.QueryNamed(context.Background(), "RepositoryFindRef", &query, variables) + return query.Repository.Ref.ID != "", err +} + func getTags(httpClient *http.Client, repo ghrepo.Interface, limit int) ([]tag, error) { path := fmt.Sprintf("repos/%s/%s/tags?per_page=%d", repo.RepoOwner(), repo.RepoName(), limit) url := ghinstance.RESTPrefix(repo.RepoHost()) + path From 3e0db567e89fdc6fde158cfabee650187a34127d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Feb 2022 17:23:39 +0100 Subject: [PATCH 5/8] Rotate our Windows signing certificates (#5196) - The certificate pfx file is now read from WINDOWS_CERT_PFX - The password to decode the pfx is in WINDOWS_CERT_PASSWORD - Quit reading from desktop-secrets repo - Switch osslsigncode to take in pfx instead of individual certs - :fire: obsolete setup scripts --- .github/workflows/releases.yml | 21 +++++++++++++------ .goreleaser.yml | 1 - script/prepare-windows-cert.sh | 19 ----------------- script/setup-windows-certificate.ps1 | 12 ----------- script/sign-windows-executable.sh | 31 ++++++++++++++-------------- script/sign.ps1 | 7 +------ 6 files changed, 31 insertions(+), 60 deletions(-) delete mode 100755 script/prepare-windows-cert.sh delete mode 100644 script/setup-windows-certificate.ps1 diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index b95c51715..6511fc71c 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -27,6 +27,13 @@ jobs: GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} - name: Install osslsigncode run: sudo apt-get install -y osslsigncode + - name: Obtain signing cert + run: | + cert="$(mktemp -t cert.XXX)" + base64 -d <<<"$CERT_CONTENTS" > "$cert" + echo "CERT_FILE=$cert" >> $GITHUB_ENV + env: + CERT_CONTENTS: ${{ secrets.WINDOWS_CERT_PFX }} - name: Run GoReleaser uses: goreleaser/goreleaser-action@v2 with: @@ -35,8 +42,7 @@ jobs: env: GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} GORELEASER_CURRENT_TAG: ${{steps.changelog.outputs.tag-name}} - GITHUB_CERT_PASSWORD: ${{secrets.GITHUB_CERT_PASSWORD}} - DESKTOP_CERT_TOKEN: ${{secrets.DESKTOP_CERT_TOKEN}} + CERT_PASSWORD: ${{secrets.WINDOWS_CERT_PASSWORD}} - name: Checkout documentation site uses: actions/checkout@v2 with: @@ -147,15 +153,18 @@ jobs: "${MSBUILD_PATH}\MSBuild.exe" ./build/windows/gh.wixproj -p:SourceDir="$PWD" -p:OutputPath="$PWD" -p:OutputName="$name" -p:ProductVersion="$version" - name: Obtain signing cert id: obtain_cert + shell: bash + run: | + base64 -d <<<"$CERT_CONTENTS" > ./cert.pfx + printf "::set-output name=cert-file::%s\n" ".\\cert.pfx" env: - DESKTOP_CERT_TOKEN: ${{ secrets.DESKTOP_CERT_TOKEN }} - run: .\script\setup-windows-certificate.ps1 + CERT_CONTENTS: ${{ secrets.WINDOWS_CERT_PFX }} - name: Sign MSI env: CERT_FILE: ${{ steps.obtain_cert.outputs.cert-file }} EXE_FILE: ${{ steps.buildmsi.outputs.msi }} - GITHUB_CERT_PASSWORD: ${{ secrets.GITHUB_CERT_PASSWORD }} - run: .\script\sign.ps1 -Certificate $env:CERT_FILE -Executable $env:EXE_FILE + CERT_PASSWORD: ${{ secrets.WINDOWS_CERT_PASSWORD }} + run: .\script\signtool sign /d "GitHub CLI" /f $env:CERT_FILE /p $env:CERT_PASSWORD /fd sha256 /tr http://timestamp.digicert.com /v $env:EXE_FILE - name: Upload MSI shell: bash run: | diff --git a/.goreleaser.yml b/.goreleaser.yml index 01c727d93..68d3dd9c9 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -9,7 +9,6 @@ before: hooks: - go mod tidy - make manpages GH_VERSION={{.Version}} - - ./script/prepare-windows-cert.sh '{{ if index .Env "GITHUB_CERT_PASSWORD" }}{{ .Env.GITHUB_CERT_PASSWORD}}{{ end }}' '{{ if index .Env "DESKTOP_CERT_TOKEN" }}{{ .Env.DESKTOP_CERT_TOKEN}}{{ end }}' builds: - <<: &build_defaults diff --git a/script/prepare-windows-cert.sh b/script/prepare-windows-cert.sh deleted file mode 100755 index d52d12b1d..000000000 --- a/script/prepare-windows-cert.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/bash -set -e - -GITHUB_CERT_PASSWORD=$1 -DESKTOP_CERT_TOKEN=$2 - -if [[ -z "$GITHUB_CERT_PASSWORD" || -z "$DESKTOP_CERT_TOKEN" ]]; then - echo "skipping windows signing prep; cert password or token not found" - exit 0 -fi - -curl \ - -H "Authorization: token $DESKTOP_CERT_TOKEN" \ - -H "Accept: application/vnd.github.v3.raw" \ - --output windows-certificate.pfx \ - https://api.github.com/repos/desktop/desktop-secrets/contents/windows-certificate.pfx - -openssl pkcs12 -in windows-certificate.pfx -nocerts -nodes -out private-key.pem -passin pass:${GITHUB_CERT_PASSWORD} -openssl pkcs12 -in windows-certificate.pfx -nokeys -nodes -out certificate.pem -passin pass:${GITHUB_CERT_PASSWORD} diff --git a/script/setup-windows-certificate.ps1 b/script/setup-windows-certificate.ps1 deleted file mode 100644 index 9238fe67b..000000000 --- a/script/setup-windows-certificate.ps1 +++ /dev/null @@ -1,12 +0,0 @@ -$scriptPath = split-path -parent $MyInvocation.MyCommand.Definition -$certFile = "$scriptPath\windows-certificate.pfx" - -$headers = New-Object "System.Collections.Generic.Dictionary[[String],[String]]" -$headers.Add("Authorization", "token $env:DESKTOP_CERT_TOKEN") -$headers.Add("Accept", 'application/vnd.github.v3.raw') - -Invoke-WebRequest 'https://api.github.com/repos/desktop/desktop-secrets/contents/windows-certificate.pfx' ` - -Headers $headers ` - -OutFile "$certFile" - -Write-Output "::set-output name=cert-file::$certFile" diff --git a/script/sign-windows-executable.sh b/script/sign-windows-executable.sh index 2141c9552..d89e6dbe4 100755 --- a/script/sign-windows-executable.sh +++ b/script/sign-windows-executable.sh @@ -1,26 +1,25 @@ #!/bin/bash set -e -if [[ ! -e certificate.pem || ! -e private-key.pem ]]; then - echo "skipping windows signing; cert or key not found" +EXE="$1" + +if [ -z "$CERT_FILE" ]; then + echo "skipping Windows code-signing; CERT_FILE not set" >&2 exit 0 fi -EXECUTABLE_PATH=$1 -ARCH="386" - -if [[ $EXECUTABLE_PATH =~ "amd64" ]]; then - ARCH="amd64" +if [ ! -f "$CERT_FILE" ]; then + echo "error Windows code-signing; file '$CERT_FILE' not found" >&2 + exit 1 fi -OUT_PATH=gh_signed-${ARCH}.exe +if [ -z "$CERT_PASSWORD" ]; then + echo "error Windows code-signing; no value for CERT_PASSWORD" >&2 + exit 1 +fi -osslsigncode sign \ - -certs certificate.pem \ - -key private-key.pem \ - -n "GitHub CLI" \ - -t http://timestamp.digicert.com \ - -in $EXECUTABLE_PATH \ - -out $OUT_PATH +osslsigncode sign -n "GitHub CLI" -t http://timestamp.digicert.com \ + -pkcs12 "$CERT_FILE" -readpass <(printf "%s" "$CERT_PASSWORD") -h sha256 \ + -in "$EXE" -out "$EXE"~ -mv $OUT_PATH $EXECUTABLE_PATH +mv "$EXE"~ "$EXE" diff --git a/script/sign.ps1 b/script/sign.ps1 index ec724f7bd..336e0204c 100644 --- a/script/sign.ps1 +++ b/script/sign.ps1 @@ -6,12 +6,7 @@ param ( Set-StrictMode -Version Latest $ErrorActionPreference = "Stop" -$thumbprint = "fb713a60a7fa79dfc03cb301ca05d4e8c1bdd431" -$passwd = $env:GITHUB_CERT_PASSWORD $ProgramName = "GitHub CLI" - $scriptPath = split-path -parent $MyInvocation.MyCommand.Definition -& $scriptPath\signtool.exe sign /d $ProgramName /f $Certificate /p $passwd ` - /sha1 $thumbprint /fd sha256 /tr http://timestamp.digicert.com /td sha256 /v ` - $Executable +& $scriptPath\signtool.exe sign /d $ProgramName /f $Certificate /p $env:CERT_PASSWORD /fd sha256 /tr http://timestamp.digicert.com /v $Executable From 6e14426dfb076350796b6f081b27399a5f4be904 Mon Sep 17 00:00:00 2001 From: Breno Silva Date: Tue, 15 Feb 2022 13:35:30 -0300 Subject: [PATCH 6/8] docs: improve `repo edit` docs (#5165) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- pkg/cmd/repo/edit/edit.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index 4761fb0b8..86ccdaab1 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -57,7 +57,19 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr - by URL, e.g. "https://github.com/OWNER/REPO" `), }, + Long: heredoc.Docf(` + Edit repository settings. + + To toggle a setting off, use the %[1]s--flag=false%[1]s syntax. + `, "`"), Args: cobra.MaximumNArgs(1), + Example: heredoc.Doc(` + # enable issues and wiki + gh repo edit --enable-issues --enable-wiki + + # disable projects + gh repo edit --enable-projects=false + `), RunE: func(cmd *cobra.Command, args []string) error { if cmd.Flags().NFlag() == 0 { return cmdutil.FlagErrorf("at least one flag is required") From 72340076ae7baaf7c691c4b949a32639679d7df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Feb 2022 18:27:58 +0100 Subject: [PATCH 7/8] tests --- pkg/cmd/release/create/create.go | 14 +- pkg/cmd/release/create/create_test.go | 245 +++++++++++++++++--------- pkg/cmd/release/create/http.go | 7 +- 3 files changed, 170 insertions(+), 96 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index bdfdf1460..ce615ff70 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -364,13 +364,6 @@ func createRun(opts *CreateOptions) error { } } - if opts.Draft && len(opts.DiscussionCategory) > 0 { - return fmt.Errorf( - "%s Discussions not supported with draft releases", - opts.IO.ColorScheme().FailureIcon(), - ) - } - params := map[string]interface{}{ "tag_name": opts.TagName, "draft": opts.Draft, @@ -418,12 +411,7 @@ func createRun(opts *CreateOptions) error { } if !opts.Draft { - params := map[string]interface{}{} - params["draft"] = false - if opts.DiscussionCategory != "" { - params["discussion_category_name"] = opts.DiscussionCategory - } - rel, err := publishRelease(httpClient, newRelease.APIURL, params) + rel, err := publishRelease(httpClient, newRelease.APIURL, opts.DiscussionCategory) if err != nil { return err } diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index f13150002..8b3e13e18 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -286,7 +286,7 @@ func Test_createRun(t *testing.T) { name string isTTY bool opts CreateOptions - wantParams interface{} + httpStubs func(t *testing.T, reg *httpmock.Registry) wantErr string wantStdout string wantStderr string @@ -301,12 +301,20 @@ func Test_createRun(t *testing.T) { BodyProvided: true, Target: "", }, - wantParams: map[string]interface{}{ - "tag_name": "v1.2.3", - "name": "The Big 1.2", - "body": "* Fixed bugs", - "draft": false, - "prerelease": false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "name": "The Big 1.2", + "body": "* Fixed bugs", + "draft": false, + "prerelease": false, + }, params) + })) }, wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", wantStderr: ``, @@ -322,13 +330,21 @@ func Test_createRun(t *testing.T) { Target: "", DiscussionCategory: "General", }, - wantParams: map[string]interface{}{ - "tag_name": "v1.2.3", - "name": "The Big 1.2", - "body": "* Fixed bugs", - "draft": false, - "prerelease": false, - "discussion_category_name": "General", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "name": "The Big 1.2", + "body": "* Fixed bugs", + "draft": false, + "prerelease": false, + "discussion_category_name": "General", + }, params) + })) }, wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", wantStderr: ``, @@ -343,11 +359,19 @@ func Test_createRun(t *testing.T) { BodyProvided: true, Target: "main", }, - wantParams: map[string]interface{}{ - "tag_name": "v1.2.3", - "draft": false, - "prerelease": false, - "target_commitish": "main", + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "draft": false, + "prerelease": false, + "target_commitish": "main", + }, params) + })) }, wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", wantStderr: ``, @@ -363,35 +387,22 @@ func Test_createRun(t *testing.T) { Draft: true, Target: "", }, - wantParams: map[string]interface{}{ - "tag_name": "v1.2.3", - "draft": true, - "prerelease": false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "draft": true, + "prerelease": false, + }, params) + })) }, wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", wantStderr: ``, }, - { - name: "discussion category for draft release", - isTTY: true, - opts: CreateOptions{ - TagName: "v1.2.3", - Name: "", - Body: "", - BodyProvided: true, - Draft: true, - Target: "", - DiscussionCategory: "general", - }, - wantParams: map[string]interface{}{ - "tag_name": "v1.2.3", - "draft": true, - "prerelease": false, - "discussion_category_name": "general", - }, - wantErr: "X Discussions not supported with draft releases", - wantStdout: "", - }, { name: "with generate notes", isTTY: true, @@ -403,11 +414,19 @@ func Test_createRun(t *testing.T) { BodyProvided: true, GenerateNotes: true, }, - wantParams: map[string]interface{}{ - "tag_name": "v1.2.3", - "draft": false, - "prerelease": false, - "generate_release_notes": true, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "draft": false, + "prerelease": false, + "generate_release_notes": true, + }, params) + })) }, wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", wantErr: "", @@ -432,10 +451,97 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, - wantParams: map[string]interface{}{ - "tag_name": "v1.2.3", - "draft": true, - "prerelease": false, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "draft": true, + "prerelease": false, + }, params) + })) + reg.Register(httpmock.REST("POST", "assets/upload"), func(req *http.Request) (*http.Response, error) { + q := req.URL.Query() + assert.Equal(t, "ball.tgz", q.Get("name")) + assert.Equal(t, "", q.Get("label")) + return &http.Response{ + StatusCode: 201, + Request: req, + Body: ioutil.NopCloser(bytes.NewBufferString(`{}`)), + Header: map[string][]string{ + "Content-Type": {"application/json"}, + }, + }, nil + }) + reg.Register(httpmock.REST("PATCH", "releases/123"), httpmock.RESTPayload(201, `{ + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3-final" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "draft": false, + }, params) + })) + }, + wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3-final\n", + wantStderr: ``, + }, + { + name: "upload files and create discussion", + isTTY: true, + opts: CreateOptions{ + TagName: "v1.2.3", + Name: "", + Body: "", + BodyProvided: true, + Draft: false, + Target: "", + Assets: []*shared.AssetForUpload{ + { + Name: "ball.tgz", + Open: func() (io.ReadCloser, error) { + return ioutil.NopCloser(bytes.NewBufferString(`TARBALL`)), nil + }, + }, + }, + DiscussionCategory: "general", + Concurrency: 1, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "draft": true, + "prerelease": false, + "discussion_category_name": "general", + }, params) + })) + reg.Register(httpmock.REST("POST", "assets/upload"), func(req *http.Request) (*http.Response, error) { + q := req.URL.Query() + assert.Equal(t, "ball.tgz", q.Get("name")) + assert.Equal(t, "", q.Get("label")) + return &http.Response{ + StatusCode: 201, + Request: req, + Body: ioutil.NopCloser(bytes.NewBufferString(`{}`)), + Header: map[string][]string{ + "Content-Type": {"application/json"}, + }, + }, nil + }) + reg.Register(httpmock.REST("PATCH", "releases/123"), httpmock.RESTPayload(201, `{ + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3-final" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "draft": false, + "discussion_category_name": "general", + }, params) + })) }, wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3-final\n", wantStderr: ``, @@ -449,15 +555,10 @@ func Test_createRun(t *testing.T) { io.SetStderrTTY(tt.isTTY) fakeHTTP := &httpmock.Registry{} - fakeHTTP.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.StatusStringResponse(201, `{ - "url": "https://api.github.com/releases/123", - "upload_url": "https://api.github.com/assets/upload", - "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" - }`)) - fakeHTTP.Register(httpmock.REST("POST", "assets/upload"), httpmock.StatusStringResponse(201, `{}`)) - fakeHTTP.Register(httpmock.REST("PATCH", "releases/123"), httpmock.StatusStringResponse(201, `{ - "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3-final" - }`)) + if tt.httpStubs != nil { + tt.httpStubs(t, fakeHTTP) + } + defer fakeHTTP.Verify(t) tt.opts.IO = io tt.opts.HttpClient = func() (*http.Client, error) { @@ -475,26 +576,6 @@ func Test_createRun(t *testing.T) { require.NoError(t, err) } - bb, err := ioutil.ReadAll(fakeHTTP.Requests[0].Body) - require.NoError(t, err) - var params interface{} - err = json.Unmarshal(bb, ¶ms) - require.NoError(t, err) - assert.Equal(t, tt.wantParams, params) - - if len(tt.opts.Assets) > 0 { - q := fakeHTTP.Requests[1].URL.Query() - assert.Equal(t, tt.opts.Assets[0].Name, q.Get("name")) - assert.Equal(t, tt.opts.Assets[0].Label, q.Get("label")) - - bb, err := ioutil.ReadAll(fakeHTTP.Requests[2].Body) - require.NoError(t, err) - var updateParams interface{} - err = json.Unmarshal(bb, &updateParams) - require.NoError(t, err) - assert.Equal(t, map[string]interface{}{"draft": false}, updateParams) - } - assert.Equal(t, tt.wantStdout, stdout.String()) assert.Equal(t, tt.wantStderr, stderr.String()) }) diff --git a/pkg/cmd/release/create/http.go b/pkg/cmd/release/create/http.go index b5ee5c271..628d06113 100644 --- a/pkg/cmd/release/create/http.go +++ b/pkg/cmd/release/create/http.go @@ -134,7 +134,12 @@ func createRelease(httpClient *http.Client, repo ghrepo.Interface, params map[st return &newRelease, err } -func publishRelease(httpClient *http.Client, releaseURL string, params map[string]interface{}) (*shared.Release, error) { +func publishRelease(httpClient *http.Client, releaseURL string, discussionCategory string) (*shared.Release, error) { + params := map[string]interface{}{"draft": false} + if discussionCategory != "" { + params["discussion_category_name"] = discussionCategory + } + bodyBytes, err := json.Marshal(params) if err != nil { return nil, err From c2a9c5a74b2aafc3ad33571a9b5f6b21dd2405cd Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Tue, 15 Feb 2022 13:36:06 -0500 Subject: [PATCH 8/8] Support filtering pull requests authored by GitHub Apps (#5180) --- pkg/cmd/pr/list/list.go | 14 +++++++++++ pkg/cmd/pr/list/list_test.go | 47 ++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index acc4ffb14..319efa4f7 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -36,6 +36,7 @@ type ListOptions struct { HeadBranch string Labels []string Author string + AppAuthor string Assignee string Search string Draft string @@ -82,6 +83,14 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman opts.Draft = strconv.FormatBool(draft) } + if err := cmdutil.MutuallyExclusive( + "specify only `--author` or `--app`", + opts.Author != "", + opts.AppAuthor != "", + ); err != nil { + return err + } + if runF != nil { return runF(opts) } @@ -99,6 +108,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.HeadBranch, "head", "H", "", "Filter by head branch") cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Filter by labels") cmd.Flags().StringVarP(&opts.Author, "author", "A", "", "Filter by author") + cmd.Flags().StringVar(&opts.AppAuthor, "app", "", "Filter by GitHub App author") cmd.Flags().StringVarP(&opts.Assignee, "assignee", "a", "", "Filter by assignee") cmd.Flags().StringVarP(&opts.Search, "search", "S", "", "Search pull requests with `query`") cmd.Flags().BoolVarP(&draft, "draft", "d", false, "Filter by draft state") @@ -163,6 +173,10 @@ func listRun(opts *ListOptions) error { return opts.Browser.Browse(openURL) } + if opts.AppAuthor != "" { + filters.Author = fmt.Sprintf("app/%s", opts.AppAuthor) + } + listResult, err := listPullRequests(httpClient, baseRepo, filters, opts.LimitResults) if err != nil { return err diff --git a/pkg/cmd/pr/list/list_test.go b/pkg/cmd/pr/list/list_test.go index 6e340510c..a2db7eb04 100644 --- a/pkg/cmd/pr/list/list_test.go +++ b/pkg/cmd/pr/list/list_test.go @@ -229,6 +229,53 @@ func TestPRList_filteringDraft(t *testing.T) { } } +func TestPRList_filteringAuthor(t *testing.T) { + tests := []struct { + name string + cli string + expectedQuery string + }{ + { + name: "author @me", + cli: `--author "@me"`, + expectedQuery: `repo:OWNER/REPO is:pr is:open author:@me`, + }, + { + name: "author user", + cli: `--author "monalisa"`, + expectedQuery: `repo:OWNER/REPO is:pr is:open author:monalisa`, + }, + { + name: "app author", + cli: `--author "app/dependabot"`, + expectedQuery: `repo:OWNER/REPO is:pr is:open author:app/dependabot`, + }, + { + name: "app author with app option", + cli: `--app "dependabot"`, + expectedQuery: `repo:OWNER/REPO is:pr is:open author:app/dependabot`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query PullRequestSearch\b`), + httpmock.GraphQLQuery(`{}`, func(_ string, params map[string]interface{}) { + assert.Equal(t, test.expectedQuery, params["q"].(string)) + })) + + _, err := runCommand(http, true, test.cli) + if err != nil { + t.Fatal(err) + } + }) + } +} + func TestPRList_withInvalidLimitFlag(t *testing.T) { http := initFakeHTTP() defer http.Verify(t)