Add capability to filter codespaces by repo owner (#7347)

* Add capability to filter codespaces by repo owner

* Replace Flags with PersistentFlags

* Reword flag description

* Update test seed

* Add mutual exclusion

---------

Co-authored-by: Caleb Brose <5447118+cmbrose@users.noreply.github.com>
This commit is contained in:
Kousik Mitra 2023-04-27 21:12:08 +05:30 committed by GitHub
parent 50e16890f8
commit d0207a2ede
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 146 additions and 38 deletions

View file

@ -128,11 +128,18 @@ func (a *API) GetUser(ctx context.Context) (*User, error) {
return &response, nil
}
// RepositoryOwner represents owner of a repository
type RepositoryOwner struct {
Type string `json:"type"`
Login string `json:"login"`
}
// Repository represents a GitHub repository.
type Repository struct {
ID int `json:"id"`
FullName string `json:"full_name"`
DefaultBranch string `json:"default_branch"`
ID int `json:"id"`
FullName string `json:"full_name"`
DefaultBranch string `json:"default_branch"`
Owner RepositoryOwner `json:"owner"`
}
// GetRepository returns the repository associated with the given owner and name.

View file

@ -15,6 +15,7 @@ type CodespaceSelector struct {
repoName string
codespaceName string
repoOwner string
}
var errNoFilteredCodespaces = errors.New("you have no codespaces meeting the filter criteria")
@ -25,8 +26,10 @@ func AddCodespaceSelector(cmd *cobra.Command, api apiClient) *CodespaceSelector
cmd.PersistentFlags().StringVarP(&cs.codespaceName, "codespace", "c", "", "Name of the codespace")
cmd.PersistentFlags().StringVarP(&cs.repoName, "repo", "R", "", "Filter codespace selection by repository name (user/repo)")
cmd.PersistentFlags().StringVar(&cs.repoOwner, "repo-owner", "", "Filter codespace selection by repository owner (username or org)")
cmd.MarkFlagsMutuallyExclusive("codespace", "repo")
cmd.MarkFlagsMutuallyExclusive("codespace", "repo-owner")
return cs
}
@ -102,6 +105,10 @@ func (cs *CodespaceSelector) fetchCodespaces(ctx context.Context) (codespaces []
codespaces = filteredCodespaces
}
if cs.repoOwner != "" {
codespaces = filterCodespacesByRepoOwner(codespaces, cs.repoOwner)
}
if len(codespaces) == 0 {
return nil, errNoFilteredCodespaces
}

View file

@ -48,68 +48,101 @@ func TestSelectNameWithCodespaceName(t *testing.T) {
func TestFetchCodespaces(t *testing.T) {
var (
repoA1 = &api.Codespace{Name: "1", Repository: api.Repository{FullName: "mock/A"}}
repoA2 = &api.Codespace{Name: "2", Repository: api.Repository{FullName: "mock/A"}}
octocatOwner = api.RepositoryOwner{Login: "octocat"}
cliOwner = api.RepositoryOwner{Login: "cli"}
octocatA = &api.Codespace{
Name: "1",
Repository: api.Repository{FullName: "octocat/A", Owner: octocatOwner},
}
repoB1 = &api.Codespace{Name: "1", Repository: api.Repository{FullName: "mock/B"}}
octocatA2 = &api.Codespace{
Name: "2",
Repository: api.Repository{FullName: "octocat/A", Owner: octocatOwner},
}
cliA = &api.Codespace{
Name: "3",
Repository: api.Repository{FullName: "cli/A", Owner: cliOwner},
}
octocatB = &api.Codespace{
Name: "4",
Repository: api.Repository{FullName: "octocat/B", Owner: octocatOwner},
}
)
tests := []struct {
tName string
apiCodespaces []*api.Codespace
codespaceName string
repoName string
repoOwner string
wantCodespaces []*api.Codespace
wantErr error
}{
// Empty case
{
"empty", nil, "", nil, errNoCodespaces,
tName: "empty",
apiCodespaces: nil,
wantCodespaces: nil,
wantErr: errNoCodespaces,
},
// Tests with no filtering
{
"no filtering, single codespace",
[]*api.Codespace{repoA1},
"",
[]*api.Codespace{repoA1},
nil,
tName: "no filtering, single codespaces",
apiCodespaces: []*api.Codespace{octocatA},
wantCodespaces: []*api.Codespace{octocatA},
wantErr: nil,
},
{
"no filtering, multiple codespaces",
[]*api.Codespace{repoA1, repoA2, repoB1},
"",
[]*api.Codespace{repoA1, repoA2, repoB1},
nil,
tName: "no filtering, multiple codespace",
apiCodespaces: []*api.Codespace{octocatA, cliA, octocatB},
wantCodespaces: []*api.Codespace{octocatA, cliA, octocatB},
},
// Test repo filtering
{
"repo filtering, single codespace",
[]*api.Codespace{repoA1},
"mock/A",
[]*api.Codespace{repoA1},
nil,
tName: "repo name filtering, single codespace",
apiCodespaces: []*api.Codespace{octocatA},
repoName: "octocat/A",
wantCodespaces: []*api.Codespace{octocatA},
wantErr: nil,
},
{
"repo filtering, multiple codespaces",
[]*api.Codespace{repoA1, repoA2, repoB1},
"mock/A",
[]*api.Codespace{repoA1, repoA2},
nil,
tName: "repo name filtering, multiple codespace",
apiCodespaces: []*api.Codespace{octocatA, octocatA2, cliA, octocatB},
repoName: "octocat/A",
wantCodespaces: []*api.Codespace{octocatA, octocatA2},
wantErr: nil,
},
{
"repo filtering, multiple codespaces 2",
[]*api.Codespace{repoA1, repoA2, repoB1},
"mock/B",
[]*api.Codespace{repoB1},
nil,
tName: "repo name filtering, multiple codespace 2",
apiCodespaces: []*api.Codespace{octocatA, cliA, octocatB},
repoName: "octocat/B",
wantCodespaces: []*api.Codespace{octocatB},
wantErr: nil,
},
{
"repo filtering, no matches",
[]*api.Codespace{repoA1, repoA2, repoB1},
"mock/C",
nil,
errNoFilteredCodespaces,
tName: "repo name filtering, no matches",
apiCodespaces: []*api.Codespace{octocatA, cliA, octocatB},
repoName: "Unknown/unknown",
wantCodespaces: nil,
wantErr: errNoFilteredCodespaces,
},
{
tName: "repo filtering, match with repo owner",
apiCodespaces: []*api.Codespace{octocatA, octocatA2, cliA, octocatB},
repoOwner: "octocat",
wantCodespaces: []*api.Codespace{octocatA, octocatA2, octocatB},
wantErr: nil,
},
{
tName: "repo filtering, no match with repo owner",
apiCodespaces: []*api.Codespace{octocatA, cliA, octocatB},
repoOwner: "unknown",
wantCodespaces: []*api.Codespace{},
wantErr: errNoFilteredCodespaces,
},
}
@ -121,7 +154,11 @@ func TestFetchCodespaces(t *testing.T) {
},
}
cs := &CodespaceSelector{api: api, repoName: tt.repoName}
cs := &CodespaceSelector{
api: api,
repoName: tt.repoName,
repoOwner: tt.repoOwner,
}
codespaces, err := cs.fetchCodespaces(context.Background())

View file

@ -10,6 +10,7 @@ import (
"log"
"os"
"sort"
"strings"
"github.com/AlecAivazis/survey/v2"
"github.com/AlecAivazis/survey/v2/terminal"
@ -266,3 +267,14 @@ func addDeprecatedRepoShorthand(cmd *cobra.Command, target *string) error {
return nil
}
// filterCodespacesByRepoOwner filters a list of codespaces by the owner of the repository.
func filterCodespacesByRepoOwner(codespaces []*api.Codespace, repoOwner string) []*api.Codespace {
filtered := make([]*api.Codespace, 0, len(codespaces))
for _, c := range codespaces {
if strings.EqualFold(c.Repository.Owner.Login, repoOwner) {
filtered = append(filtered, c)
}
}
return filtered
}

View file

@ -23,6 +23,7 @@ type deleteOptions struct {
keepDays uint16
orgName string
userName string
repoOwner string
isInteractive bool
now func() time.Time
@ -60,10 +61,12 @@ func newDeleteCmd(app *App) *cobra.Command {
// After the admin subcommand is added (see https://github.com/cli/cli/pull/6944#issuecomment-1419553639) we can revisit this.
opts.codespaceName = selector.codespaceName
opts.repoFilter = selector.repoName
opts.repoOwner = selector.repoOwner
if opts.deleteAll && opts.repoFilter != "" {
return cmdutil.FlagErrorf("both `--all` and `--repo` is not supported")
}
if opts.orgName != "" && opts.codespaceName != "" && opts.userName == "" {
return cmdutil.FlagErrorf("using `--org` with `--codespace` requires `--user`")
}
@ -99,6 +102,9 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) {
userName = currentUser.Login
}
codespaces, fetchErr = a.apiClient.ListCodespaces(ctx, api.ListCodespacesOptions{OrgName: opts.orgName, UserName: userName})
if opts.repoOwner != "" {
codespaces = filterCodespacesByRepoOwner(codespaces, opts.repoOwner)
}
return
})
if err != nil {
@ -139,6 +145,7 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) {
if opts.repoFilter != "" && !strings.EqualFold(c.Repository.FullName, opts.repoFilter) {
continue
}
if opts.keepDays > 0 {
t, err := time.Parse(time.RFC3339, c.LastUsedAt)
if err != nil {

View file

@ -217,6 +217,44 @@ func TestDelete(t *testing.T) {
wantDeleted: []string{"monalisa-spoonknife-123"},
wantStdout: "",
},
{
name: "by repo owner",
opts: deleteOptions{
deleteAll: true,
repoOwner: "octocat",
},
codespaces: []*api.Codespace{
{
Name: "octocat-spoonknife-123",
Repository: api.Repository{
FullName: "octocat/Spoon-Knife",
Owner: api.RepositoryOwner{
Login: "octocat",
},
},
},
{
Name: "cli-robawt-abc",
Repository: api.Repository{
FullName: "cli/ROBAWT",
Owner: api.RepositoryOwner{
Login: "cli",
},
},
},
{
Name: "octocat-spoonknife-c4f3",
Repository: api.Repository{
FullName: "octocat/Spoon-Knife",
Owner: api.RepositoryOwner{
Login: "octocat",
},
},
},
},
wantDeleted: []string{"octocat-spoonknife-123", "octocat-spoonknife-c4f3"},
wantStdout: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {