Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

feat: Option to list all workspaces #416

Merged
merged 5 commits into from
Aug 12, 2021
Merged

feat: Option to list all workspaces #416

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fc3336c
Option to list all workspaces
goodspark Aug 12, 2021
36ececb
Update internal/cmd/workspaces.go
goodspark Aug 12, 2021
e670978
Fixes
goodspark Aug 12, 2021
abc1684
Update internal/cmd/workspaces.go
goodspark Aug 12, 2021
49ad450
Fix lint
deansheather Aug 12, 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
9 changes: 6 additions & 3 deletions internal/cmd/ceapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,13 @@ func getWorkspacesByProvider(ctx context.Context, client coder.Client, wpName, u
return nil, err
}

workspaces, err = filterWorkspacesByUser(ctx, client, userEmail, workspaces)
if err != nil {
return nil, err
if userEmail != "" {
workspaces, err = filterWorkspacesByUser(ctx, client, userEmail, workspaces)
if err != nil {
return nil, err
}
}

return workspaces, nil
}

Expand Down
17 changes: 13 additions & 4 deletions internal/cmd/workspaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const (

func lsWorkspacesCommand() *cobra.Command {
var (
all bool
outputFmt string
user string
provider string
Expand All @@ -85,11 +86,18 @@ func lsWorkspacesCommand() *cobra.Command {
if err != nil {
return err
}
workspaces, err := getWorkspaces(ctx, client, user)
if err != nil {
return err
var workspaces []coder.Workspace
if !all {
var err error
Copy link
Member

@deansheather deansheather Aug 12, 2021

Choose a reason for hiding this comment

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

You shouldn't need the var errs here or in the other if branches since it's already defined above, then you can condense the if err check to be underneath the if branches, so each branch is just the workspaces, err = getX() statement.

edit: check my suggestion

workspaces, err = getWorkspaces(ctx, client, user)
if err != nil {
return err
}
} else {
// If the user gave the all flag, then filtering by user doesn't make sense.
user = ""
}
if provider != "" {
if provider != "" || all {
Copy link
Member

@deansheather deansheather Aug 12, 2021

Choose a reason for hiding this comment

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

This means the --all flag only works with the --provider flag. I think that's OK for now, but it needs to be made obvious in the flag description (e.g. "requires --provider flag") and there should be a check at the top of the command to ensure --provider is set if --all is set.

$ ./coder envs ls --all
fatal: resource not found

Copy link
Contributor Author

@goodspark goodspark Aug 12, 2021

Choose a reason for hiding this comment

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

Doesn't it mean it works regardless of the provider flag? If I call coder with just --all, provider is "", and the logic becomes if "" != "" || true -> if false || true -> if true. This was my intent - as an admin, I want to see all workspaces.

But looking at the actual logic/implementation for getWorkspacesByProvider, I think your point still stands. I can fix that.

workspaces, err = getWorkspacesByProvider(ctx, client, provider, user)
if err != nil {
return err
Expand Down Expand Up @@ -124,6 +132,7 @@ func lsWorkspacesCommand() *cobra.Command {
},
}

cmd.Flags().BoolVar(&all, "all", false, "Get workspaces for all users")
cmd.Flags().StringVar(&user, "user", coder.Me, "Specify the user whose resources to target")
cmd.Flags().StringVarP(&outputFmt, "output", "o", humanOutput, "human | json")
cmd.Flags().StringVarP(&provider, "provider", "p", "", "Filter workspaces by a particular workspace provider name.")
Expand Down
28 changes: 28 additions & 0 deletions internal/cmd/workspaces_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,34 @@ func Test_workspaces_ls(t *testing.T) {
res.stdoutUnmarshals(t, &workspaces)
}

func Test_workspaces_ls_all(t *testing.T) {
skipIfNoAuth(t)
for _, test := range []struct {
name string
command []string
assert func(r result)
}{
{
name: "simple list",
command: []string{"workspaces", "ls", "--all"},
assert: func(r result) { r.success(t) },
Copy link
Contributor Author

@goodspark goodspark Aug 12, 2021

Choose a reason for hiding this comment

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

Sanity check: This isn't actually checking the output - just that the command exited with 0?

Copy link
Member

@deansheather deansheather Aug 12, 2021

Choose a reason for hiding this comment

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

Essentially, yeah. This mini test framework "runs" the commands in-process and checks if the command fn returned an error.

},
{
name: "list as json",
command: []string{"workspaces", "ls", "--all", "--output", "json"},
assert: func(r result) {
var workspaces []coder.Workspace
r.stdoutUnmarshals(t, &workspaces)
},
},
} {
test := test
t.Run(test.name, func(t *testing.T) {
test.assert(execute(t, nil, test.command...))
})
}
}

func Test_workspaces_ls_by_provider(t *testing.T) {
skipIfNoAuth(t)
for _, test := range []struct {
Expand Down