Merge pull request #1763 from Matt-Gleich/trunk

♻️ Refactor gist list to use graphQL
This commit is contained in:
Nate Smith 2020-10-05 16:15:39 -05:00 committed by GitHub
commit d63b5a9297
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 314 additions and 129 deletions

View file

@ -1,46 +1,88 @@
package list
import (
"fmt"
"context"
"net/http"
"net/url"
"sort"
"strings"
"time"
"github.com/cli/cli/api"
"github.com/cli/cli/internal/ghinstance"
"github.com/cli/cli/pkg/cmd/gist/shared"
"github.com/shurcooL/githubv4"
"github.com/shurcooL/graphql"
)
func listGists(client *http.Client, hostname string, limit int, visibility string) ([]shared.Gist, error) {
result := []shared.Gist{}
query := url.Values{}
if visibility == "all" {
query.Add("per_page", fmt.Sprintf("%d", limit))
} else {
query.Add("per_page", "100")
type response struct {
Viewer struct {
Gists struct {
Nodes []struct {
Description string
Files []struct {
Name string
}
IsPublic bool
Name string
UpdatedAt time.Time
}
PageInfo struct {
HasNextPage bool
EndCursor string
}
} `graphql:"gists(first: $per_page, after: $endCursor, privacy: $visibility, orderBy: {field: CREATED_AT, direction: DESC})"`
}
}
// TODO switch to graphql
apiClient := api.NewClientFromHTTP(client)
err := apiClient.REST(hostname, "GET", "gists?"+query.Encode(), nil, &result)
if err != nil {
return nil, err
perPage := limit
if perPage > 100 {
perPage = 100
}
variables := map[string]interface{}{
"per_page": githubv4.Int(perPage),
"endCursor": (*githubv4.String)(nil),
"visibility": githubv4.GistPrivacy(strings.ToUpper(visibility)),
}
gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client)
gists := []shared.Gist{}
pagination:
for {
var result response
err := gql.QueryNamed(context.Background(), "GistList", &result, variables)
if err != nil {
return nil, err
}
for _, gist := range result {
if len(gists) == limit {
for _, gist := range result.Viewer.Gists.Nodes {
files := map[string]*shared.GistFile{}
for _, file := range gist.Files {
files[file.Name] = &shared.GistFile{
Filename: file.Name,
}
}
gists = append(
gists,
shared.Gist{
ID: gist.Name,
Description: gist.Description,
Files: files,
UpdatedAt: gist.UpdatedAt,
Public: gist.IsPublic,
},
)
if len(gists) == limit {
break pagination
}
}
if !result.Viewer.Gists.PageInfo.HasNextPage {
break
}
if visibility == "all" || (visibility == "secret" && !gist.Public) || (visibility == "public" && gist.Public) {
gists = append(gists, gist)
}
variables["endCursor"] = githubv4.String(result.Viewer.Gists.PageInfo.EndCursor)
}
sort.SliceStable(gists, func(i, j int) bool {
return gists[i].UpdatedAt.After(gists[j].UpdatedAt)
})
return gists, nil
}

View file

@ -9,6 +9,7 @@ import (
"github.com/cli/cli/internal/ghinstance"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/pkg/text"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
)
@ -27,6 +28,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
HttpClient: f.HttpClient,
}
var flagPublic bool
var flagSecret bool
cmd := &cobra.Command{
Use: "list",
Short: "List your gists",
@ -36,27 +40,23 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
return &cmdutil.FlagError{Err: fmt.Errorf("invalid limit: %v", opts.Limit)}
}
pub := cmd.Flags().Changed("public")
secret := cmd.Flags().Changed("secret")
opts.Visibility = "all"
if pub && !secret {
opts.Visibility = "public"
} else if secret && !pub {
if flagSecret {
opts.Visibility = "secret"
} else if flagPublic {
opts.Visibility = "public"
}
if runF != nil {
return runF(opts)
}
return listRun(opts)
},
}
cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 10, "Maximum number of gists to fetch")
cmd.Flags().Bool("public", false, "Show only public gists")
cmd.Flags().Bool("secret", false, "Show only secret gists")
cmd.Flags().BoolVar(&flagPublic, "public", false, "Show only public gists")
cmd.Flags().BoolVar(&flagSecret, "secret", false, "Show only secret gists")
return cmd
}
@ -77,10 +77,7 @@ func listRun(opts *ListOptions) error {
tp := utils.NewTablePrinter(opts.IO)
for _, gist := range gists {
fileCount := 0
for range gist.Files {
fileCount++
}
fileCount := len(gist.Files)
visibility := "public"
visColor := cs.Green
@ -99,16 +96,16 @@ func listRun(opts *ListOptions) error {
}
}
gistTime := gist.UpdatedAt.Format(time.RFC3339)
if tp.IsTTY() {
gistTime = utils.FuzzyAgo(time.Since(gist.UpdatedAt))
}
tp.AddField(gist.ID, nil, nil)
tp.AddField(description, nil, cs.Bold)
tp.AddField(text.ReplaceExcessiveWhitespace(description), nil, cs.Bold)
tp.AddField(utils.Pluralize(fileCount, "file"), nil, nil)
tp.AddField(visibility, nil, visColor)
if tp.IsTTY() {
updatedAt := utils.FuzzyAgo(time.Since(gist.UpdatedAt))
tp.AddField(updatedAt, nil, cs.Gray)
} else {
tp.AddField(gist.UpdatedAt.String(), nil, nil)
}
tp.AddField(gistTime, nil, cs.Gray)
tp.EndRow()
}

View file

@ -2,11 +2,12 @@ package list
import (
"bytes"
"fmt"
"net/http"
"testing"
"time"
"github.com/cli/cli/pkg/cmd/gist/shared"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/pkg/iostreams"
@ -43,12 +44,20 @@ func TestNewCmdList(t *testing.T) {
Visibility: "secret",
},
},
{
name: "secret with explicit false value",
cli: "--secret=false",
wants: ListOptions{
Limit: 10,
Visibility: "all",
},
},
{
name: "public and secret",
cli: "--secret --public",
wants: ListOptions{
Limit: 10,
Visibility: "all",
Visibility: "secret",
},
},
{
@ -88,115 +97,252 @@ func TestNewCmdList(t *testing.T) {
}
func Test_listRun(t *testing.T) {
const query = `query GistList\b`
sixHours, _ := time.ParseDuration("6h")
sixHoursAgo := time.Now().Add(-sixHours)
absTime, _ := time.Parse(time.RFC3339, "2020-07-30T15:24:28Z")
tests := []struct {
name string
opts *ListOptions
wantOut string
stubs func(*httpmock.Registry)
nontty bool
updatedAt *time.Time
name string
opts *ListOptions
wantOut string
stubs func(*httpmock.Registry)
nontty bool
}{
{
name: "no gists",
opts: &ListOptions{},
stubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "gists"),
httpmock.JSONResponse([]shared.Gist{}))
reg.Register(
httpmock.GraphQL(query),
httpmock.StringResponse(`{ "data": { "viewer": {
"gists": { "nodes": [] }
} } }`))
},
wantOut: "",
},
{
name: "default behavior",
opts: &ListOptions{},
name: "default behavior",
opts: &ListOptions{},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(query),
httpmock.StringResponse(fmt.Sprintf(
`{ "data": { "viewer": { "gists": { "nodes": [
{
"name": "1234567890",
"files": [{ "name": "cool.txt" }],
"description": "",
"updatedAt": "%[1]v",
"isPublic": true
},
{
"name": "4567890123",
"files": [{ "name": "gistfile0.txt" }],
"description": "",
"updatedAt": "%[1]v",
"isPublic": true
},
{
"name": "2345678901",
"files": [
{ "name": "gistfile0.txt" },
{ "name": "gistfile1.txt" }
],
"description": "tea leaves thwart those who court catastrophe",
"updatedAt": "%[1]v",
"isPublic": false
},
{
"name": "3456789012",
"files": [
{ "name": "gistfile0.txt" },
{ "name": "gistfile1.txt" },
{ "name": "gistfile2.txt" },
{ "name": "gistfile3.txt" },
{ "name": "gistfile4.txt" },
{ "name": "gistfile5.txt" },
{ "name": "gistfile6.txt" },
{ "name": "gistfile7.txt" },
{ "name": "gistfile9.txt" },
{ "name": "gistfile10.txt" },
{ "name": "gistfile11.txt" }
],
"description": "short desc",
"updatedAt": "%[1]v",
"isPublic": false
}
] } } } }`,
sixHoursAgo.Format(time.RFC3339),
)),
)
},
wantOut: "1234567890 cool.txt 1 file public about 6 hours ago\n4567890123 1 file public about 6 hours ago\n2345678901 tea leaves thwart... 2 files secret about 6 hours ago\n3456789012 short desc 11 files secret about 6 hours ago\n",
},
{
name: "with public filter",
opts: &ListOptions{Visibility: "public"},
name: "with public filter",
opts: &ListOptions{Visibility: "public"},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(query),
httpmock.StringResponse(fmt.Sprintf(
`{ "data": { "viewer": { "gists": { "nodes": [
{
"name": "1234567890",
"files": [{ "name": "cool.txt" }],
"description": "",
"updatedAt": "%[1]v",
"isPublic": true
},
{
"name": "4567890123",
"files": [{ "name": "gistfile0.txt" }],
"description": "",
"updatedAt": "%[1]v",
"isPublic": true
}
] } } } }`,
sixHoursAgo.Format(time.RFC3339),
)),
)
},
wantOut: "1234567890 cool.txt 1 file public about 6 hours ago\n4567890123 1 file public about 6 hours ago\n",
},
{
name: "with secret filter",
opts: &ListOptions{Visibility: "secret"},
name: "with secret filter",
opts: &ListOptions{Visibility: "secret"},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(query),
httpmock.StringResponse(fmt.Sprintf(
`{ "data": { "viewer": { "gists": { "nodes": [
{
"name": "2345678901",
"files": [
{ "name": "gistfile0.txt" },
{ "name": "gistfile1.txt" }
],
"description": "tea leaves thwart those who court catastrophe",
"updatedAt": "%[1]v",
"isPublic": false
},
{
"name": "3456789012",
"files": [
{ "name": "gistfile0.txt" },
{ "name": "gistfile1.txt" },
{ "name": "gistfile2.txt" },
{ "name": "gistfile3.txt" },
{ "name": "gistfile4.txt" },
{ "name": "gistfile5.txt" },
{ "name": "gistfile6.txt" },
{ "name": "gistfile7.txt" },
{ "name": "gistfile9.txt" },
{ "name": "gistfile10.txt" },
{ "name": "gistfile11.txt" }
],
"description": "short desc",
"updatedAt": "%[1]v",
"isPublic": false
}
] } } } }`,
sixHoursAgo.Format(time.RFC3339),
)),
)
},
wantOut: "2345678901 tea leaves thwart... 2 files secret about 6 hours ago\n3456789012 short desc 11 files secret about 6 hours ago\n",
},
{
name: "with limit",
opts: &ListOptions{Limit: 1},
name: "with limit",
opts: &ListOptions{Limit: 1},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(query),
httpmock.StringResponse(fmt.Sprintf(
`{ "data": { "viewer": { "gists": { "nodes": [
{
"name": "1234567890",
"files": [{ "name": "cool.txt" }],
"description": "",
"updatedAt": "%v",
"isPublic": true
}
] } } } }`,
sixHoursAgo.Format(time.RFC3339),
)),
)
},
wantOut: "1234567890 cool.txt 1 file public about 6 hours ago\n",
},
{
name: "nontty output",
opts: &ListOptions{},
updatedAt: &time.Time{},
wantOut: "1234567890\tcool.txt\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n4567890123\t\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n2345678901\ttea leaves thwart those who court catastrophe\t2 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n3456789012\tshort desc\t11 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n",
nontty: true,
name: "nontty output",
opts: &ListOptions{},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(query),
httpmock.StringResponse(fmt.Sprintf(
`{ "data": { "viewer": { "gists": { "nodes": [
{
"name": "1234567890",
"files": [{ "name": "cool.txt" }],
"description": "",
"updatedAt": "%[1]v",
"isPublic": true
},
{
"name": "4567890123",
"files": [{ "name": "gistfile0.txt" }],
"description": "",
"updatedAt": "%[1]v",
"isPublic": true
},
{
"name": "2345678901",
"files": [
{ "name": "gistfile0.txt" },
{ "name": "gistfile1.txt" }
],
"description": "tea leaves thwart those who court catastrophe",
"updatedAt": "%[1]v",
"isPublic": false
},
{
"name": "3456789012",
"files": [
{ "name": "gistfile0.txt" },
{ "name": "gistfile1.txt" },
{ "name": "gistfile2.txt" },
{ "name": "gistfile3.txt" },
{ "name": "gistfile4.txt" },
{ "name": "gistfile5.txt" },
{ "name": "gistfile6.txt" },
{ "name": "gistfile7.txt" },
{ "name": "gistfile9.txt" },
{ "name": "gistfile10.txt" },
{ "name": "gistfile11.txt" }
],
"description": "short desc",
"updatedAt": "%[1]v",
"isPublic": false
}
] } } } }`,
absTime.Format(time.RFC3339),
)),
)
},
wantOut: heredoc.Doc(`
1234567890 cool.txt 1 file public 2020-07-30T15:24:28Z
4567890123 1 file public 2020-07-30T15:24:28Z
2345678901 tea leaves thwart those who court catastrophe 2 files secret 2020-07-30T15:24:28Z
3456789012 short desc 11 files secret 2020-07-30T15:24:28Z
`),
nontty: true,
},
}
for _, tt := range tests {
sixHoursAgo, _ := time.ParseDuration("-6h")
updatedAt := time.Now().Add(sixHoursAgo)
if tt.updatedAt != nil {
updatedAt = *tt.updatedAt
}
reg := &httpmock.Registry{}
if tt.stubs == nil {
reg.Register(httpmock.REST("GET", "gists"),
httpmock.JSONResponse([]shared.Gist{
{
ID: "1234567890",
UpdatedAt: updatedAt,
Description: "",
Files: map[string]*shared.GistFile{
"cool.txt": {},
},
Public: true,
},
{
ID: "4567890123",
UpdatedAt: updatedAt,
Description: "",
Files: map[string]*shared.GistFile{
"gistfile0.txt": {},
},
Public: true,
},
{
ID: "2345678901",
UpdatedAt: updatedAt,
Description: "tea leaves thwart those who court catastrophe",
Files: map[string]*shared.GistFile{
"gistfile0.txt": {},
"gistfile1.txt": {},
},
Public: false,
},
{
ID: "3456789012",
UpdatedAt: updatedAt,
Description: "short desc",
Files: map[string]*shared.GistFile{
"gistfile0.txt": {},
"gistfile1.txt": {},
"gistfile2.txt": {},
"gistfile3.txt": {},
"gistfile4.txt": {},
"gistfile5.txt": {},
"gistfile6.txt": {},
"gistfile7.txt": {},
"gistfile8.txt": {},
"gistfile9.txt": {},
"gistfile10.txt": {},
},
Public: false,
},
}))
} else {
tt.stubs(reg)
}
tt.stubs(reg)
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil