Skip to content

Commit 6ea0f72

Browse files
riac03riac03
authored
fix: brev ls all showing all instances in org (#401)
* fix: brev ls all showing all instances in org * fix: test for external nodes
1 parent a62f179 commit 6ea0f72

2 files changed

Lines changed: 132 additions & 53 deletions

File tree

‎pkg/cmd/ls/ls.go‎

Lines changed: 13 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"github.com/brevdev/brev-cli/pkg/cmdcontext"
2525
"github.com/brevdev/brev-cli/pkg/config"
2626
"github.com/brevdev/brev-cli/pkg/entity"
27-
"github.com/brevdev/brev-cli/pkg/entity/virtualproject"
2827
breverrors "github.com/brevdev/brev-cli/pkg/errors"
2928
"github.com/brevdev/brev-cli/pkg/featureflag"
3029
"github.com/brevdev/brev-cli/pkg/store"
@@ -383,27 +382,14 @@ func (ls Ls) RunUser(_ bool) error {
383382
return nil
384383
}
385384

386-
func (ls Ls) ShowAllWorkspaces(org *entity.Organization, otherOrgs []entity.Organization, user *entity.User, allWorkspaces []entity.Workspace, gpuLookup map[string]string) {
387-
userWorkspaces := store.FilterForUserWorkspaces(allWorkspaces, user.ID)
388-
ls.displayWorkspacesAndHelp(org, otherOrgs, userWorkspaces, allWorkspaces, gpuLookup)
389-
390-
projects := virtualproject.NewVirtualProjects(allWorkspaces)
391-
392-
var unjoinedProjects []virtualproject.VirtualProject
393-
for _, p := range projects {
394-
wks := p.GetUserWorkspaces(user.ID)
395-
if len(wks) == 0 {
396-
unjoinedProjects = append(unjoinedProjects, p)
397-
}
398-
}
399-
400-
displayProjects(ls.terminal, org.Name, unjoinedProjects)
385+
func (ls Ls) ShowAllWorkspaces(org *entity.Organization, otherOrgs []entity.Organization, workspacesToShow []entity.Workspace, gpuLookup map[string]string) {
386+
ls.displayWorkspacesAndHelp(org, otherOrgs, workspacesToShow, workspacesToShow, true, gpuLookup)
401387
}
402388

403389
func (ls Ls) ShowUserWorkspaces(org *entity.Organization, otherOrgs []entity.Organization, user *entity.User, allWorkspaces []entity.Workspace, gpuLookup map[string]string) {
404390
userWorkspaces := store.FilterForUserWorkspaces(allWorkspaces, user.ID)
405391

406-
ls.displayWorkspacesAndHelp(org, otherOrgs, userWorkspaces, allWorkspaces, gpuLookup)
392+
ls.displayWorkspacesAndHelp(org, otherOrgs, userWorkspaces, allWorkspaces, false, gpuLookup)
407393
}
408394

409395
func (ls Ls) ShowOrgWorkspaces(org *entity.Organization, workspaces []entity.Workspace, gpuLookup map[string]string) {
@@ -417,10 +403,10 @@ func (ls Ls) ShowOrgWorkspaces(org *entity.Organization, workspaces []entity.Wor
417403
fmt.Print("\n")
418404
}
419405

420-
func (ls Ls) displayWorkspacesAndHelp(org *entity.Organization, otherOrgs []entity.Organization, userWorkspaces []entity.Workspace, allWorkspaces []entity.Workspace, gpuLookup map[string]string) {
421-
if len(userWorkspaces) == 0 {
406+
func (ls Ls) displayWorkspacesAndHelp(org *entity.Organization, otherOrgs []entity.Organization, workspacesToDisplay []entity.Workspace, allWorkspaces []entity.Workspace, showAll bool, gpuLookup map[string]string) {
407+
if len(workspacesToDisplay) == 0 {
422408
ls.terminal.Vprint(ls.terminal.Yellow("No instances in org %s\n", org.Name))
423-
if len(allWorkspaces) > 0 {
409+
if !showAll && len(allWorkspaces) > 0 {
424410
ls.terminal.Vprintf("%s", ls.terminal.Green("See teammates' instances:\n"))
425411
ls.terminal.Vprintf("%s", ls.terminal.Yellow("\tbrev ls --all\n"))
426412
} else {
@@ -432,8 +418,12 @@ func (ls Ls) displayWorkspacesAndHelp(org *entity.Organization, otherOrgs []enti
432418
ls.terminal.Vprintf("%s", ls.terminal.Yellow(fmt.Sprintf("\tbrev set %s\n", getOtherOrg(otherOrgs, *org).Name)))
433419
}
434420
} else {
435-
ls.terminal.Vprintf("You have %d instances in Org %s\n", len(userWorkspaces), ls.terminal.Yellow(org.Name))
436-
displayWorkspacesTable(ls.terminal, userWorkspaces, gpuLookup)
421+
if showAll {
422+
ls.terminal.Vprintf("%d instances in Org %s\n", len(workspacesToDisplay), ls.terminal.Yellow(org.Name))
423+
} else {
424+
ls.terminal.Vprintf("You have %d instances in Org %s\n", len(workspacesToDisplay), ls.terminal.Yellow(org.Name))
425+
}
426+
displayWorkspacesTable(ls.terminal, workspacesToDisplay, gpuLookup)
437427

438428
fmt.Print("\n")
439429
}
@@ -530,7 +520,7 @@ func (ls Ls) RunWorkspaces(cliAuth auth.CLIAuth, org *entity.Organization, showA
530520
return breverrors.WrapAndTrace(err)
531521
}
532522
if showAll {
533-
ls.ShowAllWorkspaces(org, orgs, user, allWorkspaces, gpuLookup)
523+
ls.ShowAllWorkspaces(org, orgs, workspacesToShow, gpuLookup)
534524
if len(nodes) > 0 {
535525
ls.terminal.Vprintf("\nYou have %d external node(s) in Org %s\n", len(nodes), ls.terminal.Yellow(org.Name))
536526
displayNodesTable(ls.terminal, nodes, ls.piped)
@@ -662,23 +652,6 @@ func (ls Ls) RunHosts(org *entity.Organization) error {
662652
return nil
663653
}
664654

665-
func displayProjects(t *terminal.Terminal, orgName string, projects []virtualproject.VirtualProject) {
666-
if len(projects) > 0 {
667-
fmt.Print("\n")
668-
t.Vprintf("%d other projects in Org %s\n", len(projects), t.Yellow(orgName))
669-
displayProjectsTable(projects)
670-
671-
fmt.Print("\n")
672-
t.Vprintf("%s", t.Green("Join a project:\n")+
673-
t.Yellow(fmt.Sprintf("\tbrev start %s\n", projects[0].Name)))
674-
} else {
675-
t.Vprintf("no other projects in Org %s\n", t.Yellow(orgName))
676-
fmt.Print("\n")
677-
t.Vprintf("%s", t.Green("Invite a teamate:\n")+
678-
t.Yellow("\tbrev invite"))
679-
}
680-
}
681-
682655
func getBrevTableOptions() table.Options {
683656
options := table.OptionsDefault
684657
options.DrawBorder = false
@@ -775,19 +748,6 @@ func displayOrgTable(t *terminal.Terminal, orgs []entity.Organization, currentOr
775748
ta.Render()
776749
}
777750

778-
func displayProjectsTable(projects []virtualproject.VirtualProject) {
779-
ta := table.NewWriter()
780-
ta.SetOutputMirror(os.Stdout)
781-
ta.Style().Options = getBrevTableOptions()
782-
header := table.Row{"NAME", "MEMBERS"}
783-
ta.AppendHeader(header)
784-
for _, p := range projects {
785-
workspaceRow := []table.Row{{p.Name, p.GetUniqueUserCount()}}
786-
ta.AppendRows(workspaceRow)
787-
}
788-
ta.Render()
789-
}
790-
791751
func getStatusColoredText(t *terminal.Terminal, status string) string {
792752
switch status {
793753
case entity.Running, entity.Ready, string(entity.Completed):

‎pkg/cmd/ls/ls_test.go‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
package ls
22

33
import (
4+
"context"
45
"encoding/json"
6+
"net/http/httptest"
57
"os"
68
"strings"
79
"testing"
810

11+
nodev1connect "buf.build/gen/go/brevdev/devplane/connectrpc/go/devplaneapi/v1/devplaneapiv1connect"
912
nodev1 "buf.build/gen/go/brevdev/devplane/protocolbuffers/go/devplaneapi/v1"
13+
"connectrpc.com/connect"
1014

1115
authpkg "github.com/brevdev/brev-cli/pkg/auth"
1216
"github.com/brevdev/brev-cli/pkg/cmd/gpusearch"
@@ -401,6 +405,41 @@ func TestRunLs_OrgsJSON(t *testing.T) {
401405
}
402406
}
403407

408+
// TestRunLs_ShowAllTable verifies that --all lists every org instance in table output.
409+
func TestRunLs_ShowAllTable(t *testing.T) {
410+
s := newTestStore()
411+
s.workspaces = []entity.Workspace{
412+
{
413+
ID: "ws-mine",
414+
Name: "my-ws",
415+
Status: entity.Running,
416+
VerbBuildStatus: entity.Completed,
417+
CreatedByUserID: "u1",
418+
},
419+
{
420+
ID: "ws-other",
421+
Name: "other-ws",
422+
Status: entity.Running,
423+
VerbBuildStatus: entity.Completed,
424+
CreatedByUserID: "u2",
425+
},
426+
}
427+
term := terminal.New()
428+
429+
out := captureStdout(t, func() {
430+
err := RunLs(term, resolveTestCLIAuth(t, s), s, nil, "", true, false)
431+
if err != nil {
432+
t.Fatalf("RunLs --all returned error: %v", err)
433+
}
434+
})
435+
if !strings.Contains(out, "my-ws") || !strings.Contains(out, "other-ws") {
436+
t.Fatalf("expected both workspaces in table output, got:\n%s", out)
437+
}
438+
if strings.Contains(out, "brev ls --all") {
439+
t.Fatal("should not suggest brev ls --all when already using --all")
440+
}
441+
}
442+
404443
// TestRunLs_ShowAll verifies that --all includes workspaces from other users.
405444
func TestRunLs_ShowAllJSON(t *testing.T) {
406445
s := newTestStore()
@@ -673,3 +712,83 @@ func TestOutputWorkspacesJSON_NoNodes(t *testing.T) {
673712
t.Error("nil nodes should not produce a nodes key in the output")
674713
}
675714
}
715+
716+
type lsFakeNodeService struct {
717+
nodev1connect.UnimplementedExternalNodeServiceHandler
718+
}
719+
720+
func (f *lsFakeNodeService) ListNodes(_ context.Context, req *connect.Request[nodev1.ListNodesRequest]) (*connect.Response[nodev1.ListNodesResponse], error) {
721+
if req.Msg.GetOrganizationId() != "org1" {
722+
return connect.NewResponse(&nodev1.ListNodesResponse{}), nil
723+
}
724+
return connect.NewResponse(&nodev1.ListNodesResponse{
725+
Items: []*nodev1.ExternalNode{
726+
{Name: "gpu-node-1", ExternalNodeId: "en-123", OrganizationId: "org1"},
727+
{Name: "gpu-node-2", ExternalNodeId: "en-456", OrganizationId: "org1"},
728+
},
729+
}), nil
730+
}
731+
732+
func withFakeNodeAPI(t *testing.T, fn func()) {
733+
t.Helper()
734+
_, handler := nodev1connect.NewExternalNodeServiceHandler(&lsFakeNodeService{})
735+
server := httptest.NewServer(handler)
736+
t.Cleanup(server.Close)
737+
t.Setenv("BREV_PUBLIC_API_URL", server.URL)
738+
fn()
739+
}
740+
741+
// TestRunLs_ShowAllJSON_WithNodes verifies --all includes nodes from listNodes.
742+
func TestRunLs_ShowAllJSON_WithNodes(t *testing.T) {
743+
term := terminal.New()
744+
745+
t.Run("matching org returns nodes", func(t *testing.T) {
746+
s := newTestStore()
747+
s.workspaces = []entity.Workspace{
748+
{ID: "ws1", Name: "my-ws", Status: entity.Running, VerbBuildStatus: entity.Completed, CreatedByUserID: "u1"},
749+
}
750+
withFakeNodeAPI(t, func() {
751+
out := captureStdout(t, func() {
752+
if err := runLs(t, term, s, nil, true); err != nil {
753+
t.Fatalf("RunLs --all: %v", err)
754+
}
755+
})
756+
var res struct {
757+
Workspaces []WorkspaceInfo `json:"workspaces"`
758+
Nodes []NodeInfo `json:"nodes"`
759+
}
760+
if err := json.Unmarshal([]byte(out), &res); err != nil {
761+
t.Fatalf("parse JSON: %v\n%s", err, out)
762+
}
763+
if len(res.Nodes) != 2 || res.Nodes[0].Name != "gpu-node-1" {
764+
t.Fatalf("expected 2 nodes from listNodes, got %+v", res.Nodes)
765+
}
766+
})
767+
})
768+
769+
t.Run("other org returns no nodes", func(t *testing.T) {
770+
s := newTestStore()
771+
s.org = &entity.Organization{ID: "other-org", Name: "other-org"}
772+
s.orgs = []entity.Organization{*s.org}
773+
s.workspaces = []entity.Workspace{
774+
{ID: "ws1", Name: "my-ws", Status: entity.Running, VerbBuildStatus: entity.Completed, CreatedByUserID: "u1"},
775+
}
776+
withFakeNodeAPI(t, func() {
777+
out := captureStdout(t, func() {
778+
if err := runLs(t, term, s, nil, true); err != nil {
779+
t.Fatalf("RunLs --all: %v", err)
780+
}
781+
})
782+
var res struct {
783+
Workspaces []WorkspaceInfo `json:"workspaces"`
784+
Nodes []NodeInfo `json:"nodes"`
785+
}
786+
if err := json.Unmarshal([]byte(out), &res); err != nil {
787+
t.Fatalf("parse JSON: %v\n%s", err, out)
788+
}
789+
if len(res.Nodes) != 0 {
790+
t.Fatalf("expected 0 nodes for non-org1 org, got %+v", res.Nodes)
791+
}
792+
})
793+
})
794+
}

0 commit comments

Comments
 (0)