From 5f516ed135118ee721021c5722b62f3ea5c8cd2e Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 6 May 2025 16:00:16 +0200 Subject: [PATCH 01/46] feat: improve coder connect tunnel handling on reconnect (#17598) Closes https://github.com/coder/internal/issues/563 The [Coder Connect tunnel](https://github.com/coder/coder/blob/main/vpn/tunnel.go) receives workspace state from the Coder server over a [dRPC stream.](https://github.com/coder/coder/blob/114ba4593b2a82dfd41cdcb7fd6eb70d866e7b86/tailnet/controllers.go#L1029) When first connecting to this stream, the current state of the user's workspaces is received, with subsequent messages being diffs on top of that state. However, if the client disconnects from this stream, such as when the user's device is suspended, and then reconnects later, no mechanism exists for the tunnel to differentiate that message containing the entire initial state from another diff, and so that state is incorrectly applied as a diff. In practice: - Tunnel connects, receives a workspace update containing all the existing workspaces & agents. - Tunnel loses connection, but isn't completely stopped. - All the user's workspaces are restarted, producing a new set of agents. - Tunnel regains connection, and receives a workspace update containing all the existing workspaces & agents. - This initial update is incorrectly applied as a diff, with the Tunnel's state containing both the old & new agents. This PR introduces a solution in which tunnelUpdater, when created, sends a FreshState flag with the WorkspaceUpdate type. This flag is handled in the vpn tunnel in the following fashion: - Preserve existing Agents - Remove current Agents in the tunnel that are not present in the WorkspaceUpdate - Remove unreferenced Workspaces --- tailnet/controllers.go | 32 ++- tailnet/controllers_test.go | 9 +- vpn/tunnel.go | 77 +++++-- vpn/tunnel_internal_test.go | 396 ++++++++++++++++++++++++++++++++++++ 4 files changed, 498 insertions(+), 16 deletions(-) diff --git a/tailnet/controllers.go b/tailnet/controllers.go index 2328e19640a4d..b7d4e246a4bee 100644 --- a/tailnet/controllers.go +++ b/tailnet/controllers.go @@ -897,6 +897,21 @@ type Workspace struct { agents map[uuid.UUID]*Agent } +func (w *Workspace) Clone() Workspace { + agents := make(map[uuid.UUID]*Agent, len(w.agents)) + for k, v := range w.agents { + clone := v.Clone() + agents[k] = &clone + } + return Workspace{ + ID: w.ID, + Name: w.Name, + Status: w.Status, + ownerUsername: w.ownerUsername, + agents: agents, + } +} + type DNSNameOptions struct { Suffix string } @@ -1049,6 +1064,7 @@ func (t *tunnelUpdater) recvLoop() { t.logger.Debug(context.Background(), "tunnel updater recvLoop started") defer t.logger.Debug(context.Background(), "tunnel updater recvLoop done") defer close(t.recvLoopDone) + updateKind := Snapshot for { update, err := t.client.Recv() if err != nil { @@ -1061,8 +1077,10 @@ func (t *tunnelUpdater) recvLoop() { } t.logger.Debug(context.Background(), "got workspace update", slog.F("workspace_update", update), + slog.F("update_kind", updateKind), ) - err = t.handleUpdate(update) + err = t.handleUpdate(update, updateKind) + updateKind = Diff if err != nil { t.logger.Critical(context.Background(), "failed to handle workspace Update", slog.Error(err)) cErr := t.client.Close() @@ -1083,14 +1101,23 @@ type WorkspaceUpdate struct { UpsertedAgents []*Agent DeletedWorkspaces []*Workspace DeletedAgents []*Agent + Kind UpdateKind } +type UpdateKind int + +const ( + Diff UpdateKind = iota + Snapshot +) + func (w *WorkspaceUpdate) Clone() WorkspaceUpdate { clone := WorkspaceUpdate{ UpsertedWorkspaces: make([]*Workspace, len(w.UpsertedWorkspaces)), UpsertedAgents: make([]*Agent, len(w.UpsertedAgents)), DeletedWorkspaces: make([]*Workspace, len(w.DeletedWorkspaces)), DeletedAgents: make([]*Agent, len(w.DeletedAgents)), + Kind: w.Kind, } for i, ws := range w.UpsertedWorkspaces { clone.UpsertedWorkspaces[i] = &Workspace{ @@ -1115,7 +1142,7 @@ func (w *WorkspaceUpdate) Clone() WorkspaceUpdate { return clone } -func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate) error { +func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate, updateKind UpdateKind) error { t.Lock() defer t.Unlock() @@ -1124,6 +1151,7 @@ func (t *tunnelUpdater) handleUpdate(update *proto.WorkspaceUpdate) error { UpsertedAgents: []*Agent{}, DeletedWorkspaces: []*Workspace{}, DeletedAgents: []*Agent{}, + Kind: updateKind, } for _, uw := range update.UpsertedWorkspaces { diff --git a/tailnet/controllers_test.go b/tailnet/controllers_test.go index 67834de462655..bb5b543378ebe 100644 --- a/tailnet/controllers_test.go +++ b/tailnet/controllers_test.go @@ -1611,6 +1611,7 @@ func TestTunnelAllWorkspaceUpdatesController_Initial(t *testing.T) { }, DeletedWorkspaces: []*tailnet.Workspace{}, DeletedAgents: []*tailnet.Agent{}, + Kind: tailnet.Snapshot, } // And the callback @@ -1626,6 +1627,9 @@ func TestTunnelAllWorkspaceUpdatesController_Initial(t *testing.T) { slices.SortFunc(recvState.UpsertedAgents, func(a, b *tailnet.Agent) int { return strings.Compare(a.Name, b.Name) }) + // tunnel is still open, so it's a diff + currentState.Kind = tailnet.Diff + require.Equal(t, currentState, recvState) } @@ -1692,14 +1696,17 @@ func TestTunnelAllWorkspaceUpdatesController_DeleteAgent(t *testing.T) { }, DeletedWorkspaces: []*tailnet.Workspace{}, DeletedAgents: []*tailnet.Agent{}, + Kind: tailnet.Snapshot, } cbUpdate := testutil.TryReceive(ctx, t, fUH.ch) require.Equal(t, initRecvUp, cbUpdate) - // Current state should match initial state, err := updateCtrl.CurrentState() require.NoError(t, err) + // tunnel is still open, so it's a diff + initRecvUp.Kind = tailnet.Diff + require.Equal(t, initRecvUp, state) // Send update that removes w1a1 and adds w1a2 diff --git a/vpn/tunnel.go b/vpn/tunnel.go index 63de203980d14..6c71aecaa0965 100644 --- a/vpn/tunnel.go +++ b/vpn/tunnel.go @@ -88,6 +88,7 @@ func NewTunnel( netLoopDone: make(chan struct{}), uSendCh: s.sendCh, agents: map[uuid.UUID]tailnet.Agent{}, + workspaces: map[uuid.UUID]tailnet.Workspace{}, clock: quartz.NewReal(), }, } @@ -347,7 +348,9 @@ type updater struct { uSendCh chan<- *TunnelMessage // agents contains the agents that are currently connected to the tunnel. agents map[uuid.UUID]tailnet.Agent - conn Conn + // workspaces contains the workspaces to which agents are currently connected via the tunnel. + workspaces map[uuid.UUID]tailnet.Workspace + conn Conn clock quartz.Clock } @@ -397,6 +400,11 @@ func (u *updater) sendUpdateResponse(req *request[*TunnelMessage, *ManagerMessag // createPeerUpdateLocked creates a PeerUpdate message from a workspace update, populating // the network status of the agents. func (u *updater) createPeerUpdateLocked(update tailnet.WorkspaceUpdate) *PeerUpdate { + // if the update is a snapshot, we need to process the full state + if update.Kind == tailnet.Snapshot { + processSnapshotUpdate(&update, u.agents, u.workspaces) + } + out := &PeerUpdate{ UpsertedWorkspaces: make([]*Workspace, len(update.UpsertedWorkspaces)), UpsertedAgents: make([]*Agent, len(update.UpsertedAgents)), @@ -404,7 +412,20 @@ func (u *updater) createPeerUpdateLocked(update tailnet.WorkspaceUpdate) *PeerUp DeletedAgents: make([]*Agent, len(update.DeletedAgents)), } - u.saveUpdateLocked(update) + // save the workspace update to the tunnel's state, such that it can + // be used to populate automated peer updates. + for _, agent := range update.UpsertedAgents { + u.agents[agent.ID] = agent.Clone() + } + for _, agent := range update.DeletedAgents { + delete(u.agents, agent.ID) + } + for _, workspace := range update.UpsertedWorkspaces { + u.workspaces[workspace.ID] = workspace.Clone() + } + for _, workspace := range update.DeletedWorkspaces { + delete(u.workspaces, workspace.ID) + } for i, ws := range update.UpsertedWorkspaces { out.UpsertedWorkspaces[i] = &Workspace{ @@ -413,6 +434,7 @@ func (u *updater) createPeerUpdateLocked(update tailnet.WorkspaceUpdate) *PeerUp Status: Workspace_Status(ws.Status), } } + upsertedAgents := u.convertAgentsLocked(update.UpsertedAgents) out.UpsertedAgents = upsertedAgents for i, ws := range update.DeletedWorkspaces { @@ -472,17 +494,6 @@ func (u *updater) convertAgentsLocked(agents []*tailnet.Agent) []*Agent { return out } -// saveUpdateLocked saves the workspace update to the tunnel's state, such that it can -// be used to populate automated peer updates. -func (u *updater) saveUpdateLocked(update tailnet.WorkspaceUpdate) { - for _, agent := range update.UpsertedAgents { - u.agents[agent.ID] = agent.Clone() - } - for _, agent := range update.DeletedAgents { - delete(u.agents, agent.ID) - } -} - // setConn sets the `conn` and returns false if there's already a connection set. func (u *updater) setConn(conn Conn) bool { u.mu.Lock() @@ -552,6 +563,46 @@ func (u *updater) netStatusLoop() { } } +// processSnapshotUpdate handles the logic when a full state update is received. +// When the tunnel is live, we only receive diffs, but the first packet on any given +// reconnect to the tailnet API is a full state. +// Without this logic we weren't processing deletes for any workspaces or agents deleted +// while the client was disconnected while the computer was asleep. +func processSnapshotUpdate(update *tailnet.WorkspaceUpdate, agents map[uuid.UUID]tailnet.Agent, workspaces map[uuid.UUID]tailnet.Workspace) { + // ignoredWorkspaces is initially populated with the workspaces that are + // in the current update. Later on we populate it with the deleted workspaces too + // so that we don't send duplicate updates. Same applies to ignoredAgents. + ignoredWorkspaces := make(map[uuid.UUID]struct{}, len(update.UpsertedWorkspaces)) + ignoredAgents := make(map[uuid.UUID]struct{}, len(update.UpsertedAgents)) + + for _, workspace := range update.UpsertedWorkspaces { + ignoredWorkspaces[workspace.ID] = struct{}{} + } + for _, agent := range update.UpsertedAgents { + ignoredAgents[agent.ID] = struct{}{} + } + for _, agent := range agents { + if _, present := ignoredAgents[agent.ID]; !present { + // delete any current agents that are not in the new update + update.DeletedAgents = append(update.DeletedAgents, &tailnet.Agent{ + ID: agent.ID, + Name: agent.Name, + WorkspaceID: agent.WorkspaceID, + }) + } + } + for _, workspace := range workspaces { + if _, present := ignoredWorkspaces[workspace.ID]; !present { + update.DeletedWorkspaces = append(update.DeletedWorkspaces, &tailnet.Workspace{ + ID: workspace.ID, + Name: workspace.Name, + Status: workspace.Status, + }) + ignoredWorkspaces[workspace.ID] = struct{}{} + } + } +} + // hostsToIPStrings returns a slice of all unique IP addresses in the values // of the given map. func hostsToIPStrings(hosts map[dnsname.FQDN][]netip.Addr) []string { diff --git a/vpn/tunnel_internal_test.go b/vpn/tunnel_internal_test.go index d1d7377361f79..2beba66d7a7d2 100644 --- a/vpn/tunnel_internal_test.go +++ b/vpn/tunnel_internal_test.go @@ -2,6 +2,7 @@ package vpn import ( "context" + "maps" "net" "net/netip" "net/url" @@ -292,6 +293,7 @@ func TestUpdater_createPeerUpdate(t *testing.T) { ctx: ctx, netLoopDone: make(chan struct{}), agents: map[uuid.UUID]tailnet.Agent{}, + workspaces: map[uuid.UUID]tailnet.Workspace{}, conn: newFakeConn(tailnet.WorkspaceUpdate{}, hsTime), } @@ -486,6 +488,212 @@ func TestTunnel_sendAgentUpdate(t *testing.T) { require.Equal(t, hsTime, req.msg.GetPeerUpdate().UpsertedAgents[0].LastHandshake.AsTime()) } +func TestTunnel_sendAgentUpdateReconnect(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + + mClock := quartz.NewMock(t) + + wID1 := uuid.UUID{1} + aID1 := uuid.UUID{2} + aID2 := uuid.UUID{3} + + hsTime := time.Now().Add(-time.Minute).UTC() + + client := newFakeClient(ctx, t) + conn := newFakeConn(tailnet.WorkspaceUpdate{}, hsTime) + + tun, mgr := setupTunnel(t, ctx, client, mClock) + errCh := make(chan error, 1) + var resp *TunnelMessage + go func() { + r, err := mgr.unaryRPC(ctx, &ManagerMessage{ + Msg: &ManagerMessage_Start{ + Start: &StartRequest{ + TunnelFileDescriptor: 2, + CoderUrl: "https://coder.example.com", + ApiToken: "fakeToken", + }, + }, + }) + resp = r + errCh <- err + }() + testutil.RequireSend(ctx, t, client.ch, conn) + err := testutil.TryReceive(ctx, t, errCh) + require.NoError(t, err) + _, ok := resp.Msg.(*TunnelMessage_Start) + require.True(t, ok) + + // Inform the tunnel of the initial state + err = tun.Update(tailnet.WorkspaceUpdate{ + UpsertedWorkspaces: []*tailnet.Workspace{ + { + ID: wID1, Name: "w1", Status: proto.Workspace_STARTING, + }, + }, + UpsertedAgents: []*tailnet.Agent{ + { + ID: aID1, + Name: "agent1", + WorkspaceID: wID1, + Hosts: map[dnsname.FQDN][]netip.Addr{ + "agent1.coder.": {netip.MustParseAddr("fd60:627a:a42b:0101::")}, + }, + }, + }, + }) + require.NoError(t, err) + req := testutil.TryReceive(ctx, t, mgr.requests) + require.Nil(t, req.msg.Rpc) + require.NotNil(t, req.msg.GetPeerUpdate()) + require.Len(t, req.msg.GetPeerUpdate().UpsertedAgents, 1) + require.Equal(t, aID1[:], req.msg.GetPeerUpdate().UpsertedAgents[0].Id) + + // Upsert a new agent simulating a reconnect + err = tun.Update(tailnet.WorkspaceUpdate{ + UpsertedWorkspaces: []*tailnet.Workspace{ + { + ID: wID1, Name: "w1", Status: proto.Workspace_STARTING, + }, + }, + UpsertedAgents: []*tailnet.Agent{ + { + ID: aID2, + Name: "agent2", + WorkspaceID: wID1, + Hosts: map[dnsname.FQDN][]netip.Addr{ + "agent2.coder.": {netip.MustParseAddr("fd60:627a:a42b:0101::")}, + }, + }, + }, + Kind: tailnet.Snapshot, + }) + require.NoError(t, err) + + // The new update only contains the new agent + mClock.AdvanceNext() + req = testutil.TryReceive(ctx, t, mgr.requests) + require.Nil(t, req.msg.Rpc) + peerUpdate := req.msg.GetPeerUpdate() + require.NotNil(t, peerUpdate) + require.Len(t, peerUpdate.UpsertedAgents, 1) + require.Len(t, peerUpdate.DeletedAgents, 1) + require.Len(t, peerUpdate.DeletedWorkspaces, 0) + + require.Equal(t, aID2[:], peerUpdate.UpsertedAgents[0].Id) + require.Equal(t, hsTime, peerUpdate.UpsertedAgents[0].LastHandshake.AsTime()) + + require.Equal(t, aID1[:], peerUpdate.DeletedAgents[0].Id) +} + +func TestTunnel_sendAgentUpdateWorkspaceReconnect(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + + mClock := quartz.NewMock(t) + + wID1 := uuid.UUID{1} + wID2 := uuid.UUID{2} + aID1 := uuid.UUID{3} + aID3 := uuid.UUID{4} + + hsTime := time.Now().Add(-time.Minute).UTC() + + client := newFakeClient(ctx, t) + conn := newFakeConn(tailnet.WorkspaceUpdate{}, hsTime) + + tun, mgr := setupTunnel(t, ctx, client, mClock) + errCh := make(chan error, 1) + var resp *TunnelMessage + go func() { + r, err := mgr.unaryRPC(ctx, &ManagerMessage{ + Msg: &ManagerMessage_Start{ + Start: &StartRequest{ + TunnelFileDescriptor: 2, + CoderUrl: "https://coder.example.com", + ApiToken: "fakeToken", + }, + }, + }) + resp = r + errCh <- err + }() + testutil.RequireSend(ctx, t, client.ch, conn) + err := testutil.TryReceive(ctx, t, errCh) + require.NoError(t, err) + _, ok := resp.Msg.(*TunnelMessage_Start) + require.True(t, ok) + + // Inform the tunnel of the initial state + err = tun.Update(tailnet.WorkspaceUpdate{ + UpsertedWorkspaces: []*tailnet.Workspace{ + { + ID: wID1, Name: "w1", Status: proto.Workspace_STARTING, + }, + }, + UpsertedAgents: []*tailnet.Agent{ + { + ID: aID1, + Name: "agent1", + WorkspaceID: wID1, + Hosts: map[dnsname.FQDN][]netip.Addr{ + "agent1.coder.": {netip.MustParseAddr("fd60:627a:a42b:0101::")}, + }, + }, + }, + }) + require.NoError(t, err) + req := testutil.TryReceive(ctx, t, mgr.requests) + require.Nil(t, req.msg.Rpc) + require.NotNil(t, req.msg.GetPeerUpdate()) + require.Len(t, req.msg.GetPeerUpdate().UpsertedAgents, 1) + require.Equal(t, aID1[:], req.msg.GetPeerUpdate().UpsertedAgents[0].Id) + + // Upsert a new agent with a new workspace while simulating a reconnect + err = tun.Update(tailnet.WorkspaceUpdate{ + UpsertedWorkspaces: []*tailnet.Workspace{ + { + ID: wID2, Name: "w2", Status: proto.Workspace_STARTING, + }, + }, + UpsertedAgents: []*tailnet.Agent{ + { + ID: aID3, + Name: "agent3", + WorkspaceID: wID2, + Hosts: map[dnsname.FQDN][]netip.Addr{ + "agent3.coder.": {netip.MustParseAddr("fd60:627a:a42b:0101::")}, + }, + }, + }, + Kind: tailnet.Snapshot, + }) + require.NoError(t, err) + + // The new update only contains the new agent + mClock.AdvanceNext() + req = testutil.TryReceive(ctx, t, mgr.requests) + mClock.AdvanceNext() + + require.Nil(t, req.msg.Rpc) + peerUpdate := req.msg.GetPeerUpdate() + require.NotNil(t, peerUpdate) + require.Len(t, peerUpdate.UpsertedWorkspaces, 1) + require.Len(t, peerUpdate.UpsertedAgents, 1) + require.Len(t, peerUpdate.DeletedAgents, 1) + require.Len(t, peerUpdate.DeletedWorkspaces, 1) + + require.Equal(t, wID2[:], peerUpdate.UpsertedWorkspaces[0].Id) + require.Equal(t, aID3[:], peerUpdate.UpsertedAgents[0].Id) + require.Equal(t, hsTime, peerUpdate.UpsertedAgents[0].LastHandshake.AsTime()) + + require.Equal(t, aID1[:], peerUpdate.DeletedAgents[0].Id) + require.Equal(t, wID1[:], peerUpdate.DeletedWorkspaces[0].Id) +} + //nolint:revive // t takes precedence func setupTunnel(t *testing.T, ctx context.Context, client *fakeClient, mClock quartz.Clock) (*Tunnel, *speaker[*ManagerMessage, *TunnelMessage, TunnelMessage]) { mp, tp := net.Pipe() @@ -513,3 +721,191 @@ func setupTunnel(t *testing.T, ctx context.Context, client *fakeClient, mClock q mgr.start() return tun, mgr } + +func TestProcessFreshState(t *testing.T) { + t.Parallel() + + wsID1 := uuid.New() + wsID2 := uuid.New() + wsID3 := uuid.New() + wsID4 := uuid.New() + + agentID1 := uuid.New() + agentID2 := uuid.New() + agentID3 := uuid.New() + agentID4 := uuid.New() + + agent1 := tailnet.Agent{ID: agentID1, Name: "agent1", WorkspaceID: wsID1} + agent2 := tailnet.Agent{ID: agentID2, Name: "agent2", WorkspaceID: wsID2} + agent3 := tailnet.Agent{ID: agentID3, Name: "agent3", WorkspaceID: wsID3} + agent4 := tailnet.Agent{ID: agentID4, Name: "agent4", WorkspaceID: wsID1} + + ws1 := tailnet.Workspace{ID: wsID1, Name: "ws1", Status: proto.Workspace_RUNNING} + ws2 := tailnet.Workspace{ID: wsID2, Name: "ws2", Status: proto.Workspace_RUNNING} + ws3 := tailnet.Workspace{ID: wsID3, Name: "ws3", Status: proto.Workspace_RUNNING} + ws4 := tailnet.Workspace{ID: wsID4, Name: "ws4", Status: proto.Workspace_RUNNING} + + initialAgents := map[uuid.UUID]tailnet.Agent{ + agentID1: agent1, + agentID2: agent2, + agentID4: agent4, + } + initialWorkspaces := map[uuid.UUID]tailnet.Workspace{ + wsID1: ws1, + wsID2: ws2, + } + + tests := []struct { + name string + initialAgents map[uuid.UUID]tailnet.Agent + initialWorkspaces map[uuid.UUID]tailnet.Workspace + update *tailnet.WorkspaceUpdate + expectedDelete *tailnet.WorkspaceUpdate // We only care about deletions added by the function + }{ + { + name: "NoChange", + initialAgents: initialAgents, + initialWorkspaces: initialWorkspaces, + update: &tailnet.WorkspaceUpdate{ + Kind: tailnet.Snapshot, + UpsertedWorkspaces: []*tailnet.Workspace{&ws1, &ws2}, + UpsertedAgents: []*tailnet.Agent{&agent1, &agent2, &agent4}, + DeletedWorkspaces: []*tailnet.Workspace{}, + DeletedAgents: []*tailnet.Agent{}, + }, + expectedDelete: &tailnet.WorkspaceUpdate{ // Expect no *additional* deletions + DeletedWorkspaces: []*tailnet.Workspace{}, + DeletedAgents: []*tailnet.Agent{}, + }, + }, + { + name: "AgentAdded", // Agent 3 added in update + initialAgents: initialAgents, + initialWorkspaces: initialWorkspaces, + update: &tailnet.WorkspaceUpdate{ + Kind: tailnet.Snapshot, + UpsertedWorkspaces: []*tailnet.Workspace{&ws1, &ws2, &ws3}, + UpsertedAgents: []*tailnet.Agent{&agent1, &agent2, &agent3, &agent4}, + DeletedWorkspaces: []*tailnet.Workspace{}, + DeletedAgents: []*tailnet.Agent{}, + }, + expectedDelete: &tailnet.WorkspaceUpdate{ + DeletedWorkspaces: []*tailnet.Workspace{}, + DeletedAgents: []*tailnet.Agent{}, + }, + }, + { + name: "AgentRemovedWorkspaceAlsoRemoved", // Agent 2 removed, ws2 also removed + initialAgents: initialAgents, + initialWorkspaces: initialWorkspaces, + update: &tailnet.WorkspaceUpdate{ + Kind: tailnet.Snapshot, + UpsertedWorkspaces: []*tailnet.Workspace{&ws1}, // ws2 not present + UpsertedAgents: []*tailnet.Agent{&agent1, &agent4}, // agent2 not present + DeletedWorkspaces: []*tailnet.Workspace{}, + DeletedAgents: []*tailnet.Agent{}, + }, + expectedDelete: &tailnet.WorkspaceUpdate{ + DeletedWorkspaces: []*tailnet.Workspace{ + {ID: wsID2, Name: "ws2", Status: proto.Workspace_RUNNING}, + }, // Expect ws2 to be deleted + DeletedAgents: []*tailnet.Agent{ // Expect agent2 to be deleted + {ID: agentID2, Name: "agent2", WorkspaceID: wsID2}, + }, + }, + }, + { + name: "AgentRemovedWorkspaceStays", // Agent 4 removed, but ws1 stays (due to agent1) + initialAgents: initialAgents, + initialWorkspaces: initialWorkspaces, + update: &tailnet.WorkspaceUpdate{ + Kind: tailnet.Snapshot, + UpsertedWorkspaces: []*tailnet.Workspace{&ws1, &ws2}, // ws1 still present + UpsertedAgents: []*tailnet.Agent{&agent1, &agent2}, // agent4 not present + DeletedWorkspaces: []*tailnet.Workspace{}, + DeletedAgents: []*tailnet.Agent{}, + }, + expectedDelete: &tailnet.WorkspaceUpdate{ + DeletedWorkspaces: []*tailnet.Workspace{}, // ws1 should NOT be deleted + DeletedAgents: []*tailnet.Agent{ // Expect agent4 to be deleted + {ID: agentID4, Name: "agent4", WorkspaceID: wsID1}, + }, + }, + }, + { + name: "InitialAgentsEmpty", + initialAgents: map[uuid.UUID]tailnet.Agent{}, // Start with no agents known + initialWorkspaces: map[uuid.UUID]tailnet.Workspace{}, + update: &tailnet.WorkspaceUpdate{ + Kind: tailnet.Snapshot, + UpsertedWorkspaces: []*tailnet.Workspace{&ws1, &ws2}, + UpsertedAgents: []*tailnet.Agent{&agent1, &agent2}, + DeletedWorkspaces: []*tailnet.Workspace{}, + DeletedAgents: []*tailnet.Agent{}, + }, + expectedDelete: &tailnet.WorkspaceUpdate{ // Expect no deletions added + DeletedWorkspaces: []*tailnet.Workspace{}, + DeletedAgents: []*tailnet.Agent{}, + }, + }, + { + name: "UpdateEmpty", // Snapshot says nothing exists + initialAgents: initialAgents, + initialWorkspaces: initialWorkspaces, + update: &tailnet.WorkspaceUpdate{ + Kind: tailnet.Snapshot, + UpsertedWorkspaces: []*tailnet.Workspace{}, + UpsertedAgents: []*tailnet.Agent{}, + DeletedWorkspaces: []*tailnet.Workspace{}, + DeletedAgents: []*tailnet.Agent{}, + }, + expectedDelete: &tailnet.WorkspaceUpdate{ // Expect all initial agents/workspaces to be deleted + DeletedWorkspaces: []*tailnet.Workspace{ + {ID: wsID1, Name: "ws1", Status: proto.Workspace_RUNNING}, + {ID: wsID2, Name: "ws2", Status: proto.Workspace_RUNNING}, + }, // ws1 and ws2 deleted + DeletedAgents: []*tailnet.Agent{ // agent1, agent2, agent4 deleted + {ID: agentID1, Name: "agent1", WorkspaceID: wsID1}, + {ID: agentID2, Name: "agent2", WorkspaceID: wsID2}, + {ID: agentID4, Name: "agent4", WorkspaceID: wsID1}, + }, + }, + }, + { + name: "WorkspaceWithNoAgents", // Snapshot says nothing exists + initialAgents: initialAgents, + initialWorkspaces: map[uuid.UUID]tailnet.Workspace{wsID1: ws1, wsID2: ws2, wsID4: ws4}, // ws4 has no agents + update: &tailnet.WorkspaceUpdate{ + Kind: tailnet.Snapshot, + UpsertedWorkspaces: []*tailnet.Workspace{&ws1, &ws2}, + UpsertedAgents: []*tailnet.Agent{&agent1, &agent2, &agent4}, + DeletedWorkspaces: []*tailnet.Workspace{}, + DeletedAgents: []*tailnet.Agent{}, + }, + expectedDelete: &tailnet.WorkspaceUpdate{ // Expect all initial agents/workspaces to be deleted + DeletedWorkspaces: []*tailnet.Workspace{ + {ID: wsID4, Name: "ws4", Status: proto.Workspace_RUNNING}, + }, // ws4 should be deleted + DeletedAgents: []*tailnet.Agent{}, + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + agentsCopy := make(map[uuid.UUID]tailnet.Agent) + maps.Copy(agentsCopy, tt.initialAgents) + + workspaceCopy := make(map[uuid.UUID]tailnet.Workspace) + maps.Copy(workspaceCopy, tt.initialWorkspaces) + + processSnapshotUpdate(tt.update, agentsCopy, workspaceCopy) + + require.ElementsMatch(t, tt.expectedDelete.DeletedAgents, tt.update.DeletedAgents, "DeletedAgents mismatch") + require.ElementsMatch(t, tt.expectedDelete.DeletedWorkspaces, tt.update.DeletedWorkspaces, "DeletedWorkspaces mismatch") + }) + } +} From d9b00e48495bdaee660b9548fdca7fb6829e99c9 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Tue, 6 May 2025 11:28:14 -0300 Subject: [PATCH 02/46] feat: add inline actions into workspaces table (#17636) Related to https://github.com/coder/coder/issues/17311 This PR adds inline actions in the workspaces page. It is a bit different of the [original design](https://www.figma.com/design/OR75XeUI0Z3ksqt1mHsNQw/Workspace-views?node-id=656-3979&m=dev) because I'm splitting the work into three phases that I will explain in more details in the demo. https://github.com/user-attachments/assets/6383375e-ed10-45d1-b5d5-b4421e86d158 --- site/src/api/queries/workspaces.ts | 11 +- site/src/components/Dialogs/Dialog.tsx | 9 +- site/src/hooks/usePagination.ts | 6 +- .../workspaces/WorkspaceUpdateDialogs.tsx | 155 ++++++++++++ .../workspaces/actions.ts} | 16 +- .../useWorkspacesToBeDeleted.ts | 9 +- .../pages/WorkspacePage/Workspace.stories.tsx | 4 +- site/src/pages/WorkspacePage/Workspace.tsx | 3 - .../WorkspaceActions.stories.tsx | 15 +- .../WorkspaceActions/WorkspaceActions.tsx | 18 +- .../WorkspacePage/WorkspaceReadyPage.tsx | 71 +----- .../WorkspacePage/WorkspaceTopbar.stories.tsx | 5 +- .../pages/WorkspacePage/WorkspaceTopbar.tsx | 3 - .../WorkspacesPage/WorkspacesPage.test.tsx | 4 +- .../pages/WorkspacesPage/WorkspacesPage.tsx | 18 +- .../WorkspacesPageView.stories.tsx | 4 +- .../WorkspacesPage/WorkspacesPageView.tsx | 6 + .../pages/WorkspacesPage/WorkspacesTable.tsx | 239 +++++++++++++++++- site/src/pages/WorkspacesPage/data.ts | 26 +- 19 files changed, 495 insertions(+), 127 deletions(-) create mode 100644 site/src/modules/workspaces/WorkspaceUpdateDialogs.tsx rename site/src/{pages/WorkspacePage/WorkspaceActions/constants.ts => modules/workspaces/actions.ts} (90%) diff --git a/site/src/api/queries/workspaces.ts b/site/src/api/queries/workspaces.ts index fd840d067821a..39008f5c712a3 100644 --- a/site/src/api/queries/workspaces.ts +++ b/site/src/api/queries/workspaces.ts @@ -139,13 +139,9 @@ function workspacesKey(config: WorkspacesRequest = {}) { } export function workspaces(config: WorkspacesRequest = {}) { - // Duplicates some of the work from workspacesKey, but that felt better than - // letting invisible properties sneak into the query logic - const { q, limit } = config; - return { queryKey: workspacesKey(config), - queryFn: () => API.getWorkspaces({ q, limit }), + queryFn: () => API.getWorkspaces(config), } as const satisfies QueryOptions; } @@ -281,7 +277,10 @@ const updateWorkspaceBuild = async ( build.workspace_owner_name, build.workspace_name, ); - const previousData = queryClient.getQueryData(workspaceKey) as Workspace; + const previousData = queryClient.getQueryData(workspaceKey); + if (!previousData) { + return; + } // Check if the build returned is newer than the previous build that could be // updated from web socket diff --git a/site/src/components/Dialogs/Dialog.tsx b/site/src/components/Dialogs/Dialog.tsx index cdc271697c680..532b47a1339dc 100644 --- a/site/src/components/Dialogs/Dialog.tsx +++ b/site/src/components/Dialogs/Dialog.tsx @@ -35,7 +35,14 @@ export const DialogActionButtons: FC = ({ return ( <> {onCancel && ( - )} diff --git a/site/src/hooks/usePagination.ts b/site/src/hooks/usePagination.ts index 72ea70868fb30..2ab409e85c9d8 100644 --- a/site/src/hooks/usePagination.ts +++ b/site/src/hooks/usePagination.ts @@ -9,7 +9,7 @@ export const usePagination = ({ const [searchParams, setSearchParams] = searchParamsResult; const page = searchParams.get("page") ? Number(searchParams.get("page")) : 1; const limit = DEFAULT_RECORDS_PER_PAGE; - const offset = page <= 0 ? 0 : (page - 1) * limit; + const offset = calcOffset(page, limit); const goToPage = (page: number) => { searchParams.set("page", page.toString()); @@ -23,3 +23,7 @@ export const usePagination = ({ offset, }; }; + +export const calcOffset = (page: number, limit: number) => { + return page <= 0 ? 0 : (page - 1) * limit; +}; diff --git a/site/src/modules/workspaces/WorkspaceUpdateDialogs.tsx b/site/src/modules/workspaces/WorkspaceUpdateDialogs.tsx new file mode 100644 index 0000000000000..741bc12a6539b --- /dev/null +++ b/site/src/modules/workspaces/WorkspaceUpdateDialogs.tsx @@ -0,0 +1,155 @@ +import { MissingBuildParameters } from "api/api"; +import { updateWorkspace } from "api/queries/workspaces"; +import type { + TemplateVersion, + Workspace, + WorkspaceBuild, + WorkspaceBuildParameter, +} from "api/typesGenerated"; +import { ConfirmDialog } from "components/Dialogs/ConfirmDialog/ConfirmDialog"; +import { MemoizedInlineMarkdown } from "components/Markdown/Markdown"; +import { UpdateBuildParametersDialog } from "pages/WorkspacePage/UpdateBuildParametersDialog"; +import { type FC, useState } from "react"; +import { useMutation, useQueryClient } from "react-query"; + +type UseWorkspaceUpdateOptions = { + workspace: Workspace; + latestVersion: TemplateVersion | undefined; + onSuccess?: (build: WorkspaceBuild) => void; + onError?: (error: unknown) => void; +}; + +type UseWorkspaceUpdateResult = { + update: () => void; + isUpdating: boolean; + dialogs: { + updateConfirmation: UpdateConfirmationDialogProps; + missingBuildParameters: MissingBuildParametersDialogProps; + }; +}; + +export const useWorkspaceUpdate = ({ + workspace, + latestVersion, + onSuccess, + onError, +}: UseWorkspaceUpdateOptions): UseWorkspaceUpdateResult => { + const queryClient = useQueryClient(); + const [isConfirmingUpdate, setIsConfirmingUpdate] = useState(false); + + const updateWorkspaceOptions = updateWorkspace(workspace, queryClient); + const updateWorkspaceMutation = useMutation({ + ...updateWorkspaceOptions, + onSuccess: (build: WorkspaceBuild) => { + updateWorkspaceOptions.onSuccess(build); + onSuccess?.(build); + }, + onError, + }); + + const update = () => { + setIsConfirmingUpdate(true); + }; + + const confirmUpdate = (buildParameters: WorkspaceBuildParameter[] = []) => { + updateWorkspaceMutation.mutate(buildParameters); + setIsConfirmingUpdate(false); + }; + + return { + update, + isUpdating: updateWorkspaceMutation.isLoading, + dialogs: { + updateConfirmation: { + open: isConfirmingUpdate, + onClose: () => setIsConfirmingUpdate(false), + onConfirm: () => confirmUpdate(), + latestVersion, + }, + missingBuildParameters: { + error: updateWorkspaceMutation.error, + onClose: () => { + updateWorkspaceMutation.reset(); + }, + onUpdate: (buildParameters: WorkspaceBuildParameter[]) => { + if (updateWorkspaceMutation.error instanceof MissingBuildParameters) { + confirmUpdate(buildParameters); + } + }, + }, + }, + }; +}; + +type WorkspaceUpdateDialogsProps = { + updateConfirmation: UpdateConfirmationDialogProps; + missingBuildParameters: MissingBuildParametersDialogProps; +}; + +export const WorkspaceUpdateDialogs: FC = ({ + updateConfirmation, + missingBuildParameters, +}) => { + return ( + <> + + + + ); +}; + +type UpdateConfirmationDialogProps = { + open: boolean; + onClose: () => void; + onConfirm: () => void; + latestVersion?: TemplateVersion; +}; + +const UpdateConfirmationDialog: FC = ({ + latestVersion, + ...dialogProps +}) => { + return ( + +

+ Updating your workspace will start the workspace on the latest + template version. This can{" "} + delete non-persistent data. +

+ {latestVersion?.message && ( + + {latestVersion.message} + + )} + + } + /> + ); +}; + +type MissingBuildParametersDialogProps = { + error: unknown; + onClose: () => void; + onUpdate: (buildParameters: WorkspaceBuildParameter[]) => void; +}; + +const MissingBuildParametersDialog: FC = ({ + error, + ...dialogProps +}) => { + return ( + + ); +}; diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts b/site/src/modules/workspaces/actions.ts similarity index 90% rename from site/src/pages/WorkspacePage/WorkspaceActions/constants.ts rename to site/src/modules/workspaces/actions.ts index a327f0277d4f5..6a255e2cd2c88 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts +++ b/site/src/modules/workspaces/actions.ts @@ -34,6 +34,11 @@ const actionTypes = [ export type ActionType = (typeof actionTypes)[number]; +type ActionPermissions = { + canDebug: boolean; + isOwner: boolean; +}; + type WorkspaceAbilities = { actions: readonly ActionType[]; canCancel: boolean; @@ -42,8 +47,11 @@ type WorkspaceAbilities = { export const abilitiesByWorkspaceStatus = ( workspace: Workspace, - canDebug: boolean, + permissions: ActionPermissions, ): WorkspaceAbilities => { + const hasPermissionToCancel = + workspace.template_allow_user_cancel_workspace_jobs || permissions.isOwner; + if (workspace.dormant_at) { return { actions: ["activate"], @@ -58,7 +66,7 @@ export const abilitiesByWorkspaceStatus = ( case "starting": { return { actions: ["starting"], - canCancel: true, + canCancel: hasPermissionToCancel, canAcceptJobs: false, }; } @@ -83,7 +91,7 @@ export const abilitiesByWorkspaceStatus = ( case "stopping": { return { actions: ["stopping"], - canCancel: true, + canCancel: hasPermissionToCancel, canAcceptJobs: false, }; } @@ -115,7 +123,7 @@ export const abilitiesByWorkspaceStatus = ( case "failed": { const actions: ActionType[] = ["retry"]; - if (canDebug) { + if (permissions.canDebug) { actions.push("debug"); } diff --git a/site/src/pages/TemplateSettingsPage/TemplateSchedulePage/useWorkspacesToBeDeleted.ts b/site/src/pages/TemplateSettingsPage/TemplateSchedulePage/useWorkspacesToBeDeleted.ts index 338c157f4f791..5a974d5d8fe31 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSchedulePage/useWorkspacesToBeDeleted.ts +++ b/site/src/pages/TemplateSettingsPage/TemplateSchedulePage/useWorkspacesToBeDeleted.ts @@ -1,5 +1,6 @@ import type { Template, Workspace } from "api/typesGenerated"; import { compareAsc } from "date-fns"; +import { calcOffset } from "hooks/usePagination"; import { useWorkspacesData } from "pages/WorkspacesPage/data"; import type { TemplateScheduleFormValues } from "./formHelpers"; @@ -9,9 +10,9 @@ export const useWorkspacesToGoDormant = ( fromDate: Date, ) => { const { data } = useWorkspacesData({ - page: 0, + offset: calcOffset(0, 0), limit: 0, - query: `template:${template.name}`, + q: `template:${template.name}`, }); return data?.workspaces?.filter((workspace: Workspace) => { @@ -40,9 +41,9 @@ export const useWorkspacesToBeDeleted = ( fromDate: Date, ) => { const { data } = useWorkspacesData({ - page: 0, + offset: calcOffset(0, 0), limit: 0, - query: `template:${template.name} dormant:true`, + q: `template:${template.name} dormant:true`, }); return data?.workspaces?.filter((workspace: Workspace) => { if (!workspace.dormant_at || !formValues.time_til_dormant_autodelete_ms) { diff --git a/site/src/pages/WorkspacePage/Workspace.stories.tsx b/site/src/pages/WorkspacePage/Workspace.stories.tsx index 88198bdb7b09a..7d29b02c11cb6 100644 --- a/site/src/pages/WorkspacePage/Workspace.stories.tsx +++ b/site/src/pages/WorkspacePage/Workspace.stories.tsx @@ -3,7 +3,7 @@ import type { Meta, StoryObj } from "@storybook/react"; import type { ProvisionerJobLog } from "api/typesGenerated"; import { ProxyContext, getPreferredProxy } from "contexts/ProxyContext"; import * as Mocks from "testHelpers/entities"; -import { withDashboardProvider } from "testHelpers/storybook"; +import { withAuthProvider, withDashboardProvider } from "testHelpers/storybook"; import { Workspace } from "./Workspace"; import type { WorkspacePermissions } from "./permissions"; @@ -40,8 +40,10 @@ const meta: Meta = { data: Mocks.MockListeningPortsResponse, }, ], + user: Mocks.MockUser, }, decorators: [ + withAuthProvider, withDashboardProvider, (Story) => ( = ({ buildLogs, latestVersion, permissions, - isOwner, timings, }) => { const navigate = useNavigate(); @@ -161,7 +159,6 @@ export const Workspace: FC = ({ isUpdating={isUpdating} isRestarting={isRestarting} canUpdateWorkspace={permissions.updateWorkspace} - isOwner={isOwner} template={template} permissions={permissions} latestVersion={latestVersion} diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx index d726a047b7c57..48dda92b49503 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx @@ -3,6 +3,7 @@ import { expect, userEvent, within } from "@storybook/test"; import { agentLogsKey, buildLogsKey } from "api/queries/workspaces"; import * as Mocks from "testHelpers/entities"; import { + withAuthProvider, withDashboardProvider, withDesktopViewport, } from "testHelpers/storybook"; @@ -14,7 +15,10 @@ const meta: Meta = { args: { isUpdating: false, }, - decorators: [withDashboardProvider, withDesktopViewport], + decorators: [withDashboardProvider, withDesktopViewport, withAuthProvider], + parameters: { + user: Mocks.MockUser, + }, }; export default meta; @@ -163,14 +167,15 @@ export const CancelShownForOwner: Story = { ...Mocks.MockStartingWorkspace, template_allow_user_cancel_workspace_jobs: false, }, - isOwner: true, }, }; export const CancelShownForUser: Story = { args: { workspace: Mocks.MockStartingWorkspace, - isOwner: false, + }, + parameters: { + user: Mocks.MockUser2, }, }; @@ -180,7 +185,9 @@ export const CancelHiddenForUser: Story = { ...Mocks.MockStartingWorkspace, template_allow_user_cancel_workspace_jobs: false, }, - isOwner: false, + }, + parameters: { + user: Mocks.MockUser2, }, }; diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx index 71f890cff3a5b..b65407806ed69 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx @@ -12,7 +12,12 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, } from "components/DropdownMenu/DropdownMenu"; +import { useAuthenticated } from "hooks/useAuthenticated"; import { EllipsisVertical } from "lucide-react"; +import { + type ActionType, + abilitiesByWorkspaceStatus, +} from "modules/workspaces/actions"; import { useWorkspaceDuplication } from "pages/CreateWorkspacePage/useWorkspaceDuplication"; import { type FC, Fragment, type ReactNode, useState } from "react"; import { mustUpdateWorkspace } from "utils/workspace"; @@ -31,7 +36,6 @@ import { import { DebugButton } from "./DebugButton"; import { DownloadLogsDialog } from "./DownloadLogsDialog"; import { RetryButton } from "./RetryButton"; -import { type ActionType, abilitiesByWorkspaceStatus } from "./constants"; export interface WorkspaceActionsProps { workspace: Workspace; @@ -52,7 +56,6 @@ export interface WorkspaceActionsProps { children?: ReactNode; canChangeVersions: boolean; canDebug: boolean; - isOwner: boolean; } export const WorkspaceActions: FC = ({ @@ -73,20 +76,19 @@ export const WorkspaceActions: FC = ({ isRestarting, canChangeVersions, canDebug, - isOwner, }) => { const { duplicateWorkspace, isDuplicationReady } = useWorkspaceDuplication(workspace); const [isDownloadDialogOpen, setIsDownloadDialogOpen] = useState(false); + const { user } = useAuthenticated(); + const isOwner = + user.roles.find((role) => role.name === "owner") !== undefined; const { actions, canCancel, canAcceptJobs } = abilitiesByWorkspaceStatus( workspace, - canDebug, + { canDebug, isOwner }, ); - const showCancel = - canCancel && - (workspace.template_allow_user_cancel_workspace_jobs || isOwner); const mustUpdate = mustUpdateWorkspace(workspace, canChangeVersions); const tooltipText = getTooltipText(workspace, mustUpdate, canChangeVersions); @@ -169,7 +171,7 @@ export const WorkspaceActions: FC = ({ {buttonMapping[action]} ))} - {showCancel && } + {canCancel && } = ({ throw Error("Workspace is undefined"); } - // Owner - const { user: me } = useAuthenticated(); - const isOwner = me.roles.find((role) => role.name === "owner") !== undefined; - // Debug mode const { data: deploymentValues } = useQuery({ ...deploymentConfig(), @@ -120,10 +116,10 @@ export const WorkspaceReadyPage: FC = ({ }); // Update workspace - const [isConfirmingUpdate, setIsConfirmingUpdate] = useState(false); - const updateWorkspaceMutation = useMutation( - updateWorkspace(workspace, queryClient), - ); + const workspaceUpdate = useWorkspaceUpdate({ + workspace, + latestVersion, + }); // If a user can update the template then they can force a delete // (via orphan). @@ -233,7 +229,7 @@ export const WorkspaceReadyPage: FC = ({ { @@ -248,9 +244,7 @@ export const WorkspaceReadyPage: FC = ({ handleRestart={(buildParameters) => { setConfirmingRestart({ open: true, buildParameters }); }} - handleUpdate={() => { - setIsConfirmingUpdate(true); - }} + handleUpdate={workspaceUpdate.update} handleCancel={cancelBuildMutation.mutate} handleSettings={() => navigate("settings")} handleRetry={handleRetry} @@ -280,7 +274,6 @@ export const WorkspaceReadyPage: FC = ({ sshPrefix={sshPrefixQuery.data?.hostname_prefix} template={template} buildLogs={buildLogs} - isOwner={isOwner} timings={timingsQuery.data} /> @@ -318,23 +311,6 @@ export const WorkspaceReadyPage: FC = ({ }} /> - { - updateWorkspaceMutation.reset(); - }} - onUpdate={(buildParameters) => { - if (updateWorkspaceMutation.error instanceof MissingBuildParameters) { - updateWorkspaceMutation.mutate(buildParameters); - } - }} - /> - = ({ }} /> - { - updateWorkspaceMutation.mutate(undefined); - setIsConfirmingUpdate(false); - }} - onClose={() => setIsConfirmingUpdate(false)} - title="Update workspace?" - confirmText="Update" - description={ - -

- Updating your workspace will start the workspace on the latest - template version. This can{" "} - delete non-persistent data. -

- {latestVersion?.message && ( - - {latestVersion.message} - - )} - - } - /> - { @@ -395,6 +346,8 @@ export const WorkspaceReadyPage: FC = ({ } /> + + ); }; diff --git a/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx b/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx index ce2ad840a1df0..84af8a518acd8 100644 --- a/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx @@ -10,7 +10,7 @@ import { MockUser, MockWorkspace, } from "testHelpers/entities"; -import { withDashboardProvider } from "testHelpers/storybook"; +import { withAuthProvider, withDashboardProvider } from "testHelpers/storybook"; import { WorkspaceTopbar } from "./WorkspaceTopbar"; // We want a workspace without a deadline to not pollute the screenshot. Also @@ -28,7 +28,7 @@ const baseWorkspace: Workspace = { const meta: Meta = { title: "pages/WorkspacePage/WorkspaceTopbar", component: WorkspaceTopbar, - decorators: [withDashboardProvider], + decorators: [withAuthProvider, withDashboardProvider], args: { workspace: baseWorkspace, template: MockTemplate, @@ -41,6 +41,7 @@ const meta: Meta = { chromatic: { diffThreshold: 0.6, }, + user: MockUser, }, }; diff --git a/site/src/pages/WorkspacePage/WorkspaceTopbar.tsx b/site/src/pages/WorkspacePage/WorkspaceTopbar.tsx index 2492e50e7c5d6..a39cf6d8b13ca 100644 --- a/site/src/pages/WorkspacePage/WorkspaceTopbar.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceTopbar.tsx @@ -55,7 +55,6 @@ export interface WorkspaceProps { canDebugMode: boolean; handleRetry: (buildParameters?: TypesGen.WorkspaceBuildParameter[]) => void; handleDebug: (buildParameters?: TypesGen.WorkspaceBuildParameter[]) => void; - isOwner: boolean; template: TypesGen.Template; permissions: WorkspacePermissions; latestVersion?: TypesGen.TemplateVersion; @@ -81,7 +80,6 @@ export const WorkspaceTopbar: FC = ({ canDebugMode, handleRetry, handleDebug, - isOwner, template, latestVersion, permissions, @@ -262,7 +260,6 @@ export const WorkspaceTopbar: FC = ({ canChangeVersions={canChangeVersions} isUpdating={isUpdating} isRestarting={isRestarting} - isOwner={isOwner} /> )} diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 66388eb3f7dd1..b1ad1d887e53c 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -264,7 +264,7 @@ describe("WorkspacesPage", () => { await user.click(getWorkspaceCheckbox(workspaces[0])); await user.click(getWorkspaceCheckbox(workspaces[1])); await user.click(screen.getByRole("button", { name: /actions/i })); - const stopButton = await screen.findByText(/stop/i); + const stopButton = await screen.findByRole("menuitem", { name: /stop/i }); await user.click(stopButton); await waitFor(() => { @@ -291,7 +291,7 @@ describe("WorkspacesPage", () => { await user.click(getWorkspaceCheckbox(workspaces[0])); await user.click(getWorkspaceCheckbox(workspaces[1])); await user.click(screen.getByRole("button", { name: /actions/i })); - const startButton = await screen.findByText(/start/i); + const startButton = await screen.findByRole("menuitem", { name: /start/i }); await user.click(startButton); await waitFor(() => { diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx index ba380905adda2..551c554fd5ee3 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -1,8 +1,10 @@ +import { getErrorDetail, getErrorMessage } from "api/errors"; import { workspacePermissionsByOrganization } from "api/queries/organizations"; import { templates } from "api/queries/templates"; import type { Workspace } from "api/typesGenerated"; import { useFilter } from "components/Filter/Filter"; import { useUserFilterMenu } from "components/Filter/UserFilter"; +import { displayError } from "components/GlobalSnackbar/utils"; import { useAuthenticated } from "hooks"; import { useEffectEvent } from "hooks/hookPolyfills"; import { usePagination } from "hooks/usePagination"; @@ -10,7 +12,7 @@ import { useDashboard } from "modules/dashboard/useDashboard"; import { useOrganizationsFilterMenu } from "modules/tableFiltering/options"; import { type FC, useEffect, useMemo, useState } from "react"; import { Helmet } from "react-helmet-async"; -import { useQuery } from "react-query"; +import { useQuery, useQueryClient } from "react-query"; import { useSearchParams } from "react-router-dom"; import { pageTitle } from "utils/page"; import { BatchDeleteConfirmation } from "./BatchDeleteConfirmation"; @@ -35,6 +37,7 @@ function useSafeSearchParams() { } const WorkspacesPage: FC = () => { + const queryClient = useQueryClient(); // If we use a useSearchParams for each hook, the values will not be in sync. // So we have to use a single one, centralizing the values, and pass it to // each hook. @@ -72,7 +75,7 @@ const WorkspacesPage: FC = () => { const { data, error, queryKey, refetch } = useWorkspacesData({ ...pagination, - query: filterProps.filter.query, + q: filterProps.filter.query, }); const updateWorkspace = useWorkspaceUpdate(queryKey); @@ -128,6 +131,17 @@ const WorkspacesPage: FC = () => { onUpdateAll={() => setConfirmingBatchAction("update")} onStartAll={() => batchActions.startAll(checkedWorkspaces)} onStopAll={() => batchActions.stopAll(checkedWorkspaces)} + onActionSuccess={async () => { + await queryClient.invalidateQueries({ + queryKey, + }); + }} + onActionError={(error) => { + displayError( + getErrorMessage(error, "Failed to perform action"), + getErrorDetail(error), + ); + }} /> = { data: MockBuildInfo, }, ], + user: MockUser, }, decorators: [ + withAuthProvider, withDashboardProvider, (Story) => ( Promise; + onActionError: (error: unknown) => void; } export const WorkspacesPageView: FC = ({ @@ -88,6 +90,8 @@ export const WorkspacesPageView: FC = ({ templatesFetchStatus, canCreateTemplate, canChangeVersions, + onActionSuccess, + onActionError, }) => { // Let's say the user has 5 workspaces, but tried to hit page 100, which does // not exist. In this case, the page is not valid and we want to show a better @@ -224,6 +228,8 @@ export const WorkspacesPageView: FC = ({ onCheckChange={onCheckChange} canCheckWorkspaces={canCheckWorkspaces} templates={templates} + onActionSuccess={onActionSuccess} + onActionError={onActionError} /> )} diff --git a/site/src/pages/WorkspacesPage/WorkspacesTable.tsx b/site/src/pages/WorkspacesPage/WorkspacesTable.tsx index a9d585fccf58c..2fe94e0260a8f 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesTable.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesTable.tsx @@ -2,6 +2,13 @@ import KeyboardArrowRight from "@mui/icons-material/KeyboardArrowRight"; import Star from "@mui/icons-material/Star"; import Checkbox from "@mui/material/Checkbox"; import Skeleton from "@mui/material/Skeleton"; +import { templateVersion } from "api/queries/templates"; +import { + cancelBuild, + deleteWorkspace, + startWorkspace, + stopWorkspace, +} from "api/queries/workspaces"; import type { Template, Workspace, @@ -11,7 +18,9 @@ import type { import { Avatar } from "components/Avatar/Avatar"; import { AvatarData } from "components/Avatar/AvatarData"; import { AvatarDataSkeleton } from "components/Avatar/AvatarDataSkeleton"; +import { Button } from "components/Button/Button"; import { InfoTooltip } from "components/InfoTooltip/InfoTooltip"; +import { Spinner } from "components/Spinner/Spinner"; import { Stack } from "components/Stack/Stack"; import { StatusIndicator, @@ -30,14 +39,33 @@ import { TableLoaderSkeleton, TableRowSkeleton, } from "components/TableLoader/TableLoader"; +import { + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from "components/Tooltip/Tooltip"; import dayjs from "dayjs"; import relativeTime from "dayjs/plugin/relativeTime"; +import { useAuthenticated } from "hooks"; import { useClickableTableRow } from "hooks/useClickableTableRow"; +import { BanIcon, PlayIcon, RefreshCcwIcon, SquareIcon } from "lucide-react"; import { useDashboard } from "modules/dashboard/useDashboard"; import { WorkspaceAppStatus } from "modules/workspaces/WorkspaceAppStatus/WorkspaceAppStatus"; import { WorkspaceDormantBadge } from "modules/workspaces/WorkspaceDormantBadge/WorkspaceDormantBadge"; import { WorkspaceOutdatedTooltip } from "modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip"; -import { type FC, type ReactNode, useMemo } from "react"; +import { + WorkspaceUpdateDialogs, + useWorkspaceUpdate, +} from "modules/workspaces/WorkspaceUpdateDialogs"; +import { abilitiesByWorkspaceStatus } from "modules/workspaces/actions"; +import { + type FC, + type PropsWithChildren, + type ReactNode, + useMemo, +} from "react"; +import { useMutation, useQuery, useQueryClient } from "react-query"; import { useNavigate } from "react-router-dom"; import { cn } from "utils/cn"; import { @@ -60,6 +88,8 @@ export interface WorkspacesTableProps { canCheckWorkspaces: boolean; templates?: Template[]; canCreateTemplate: boolean; + onActionSuccess: () => Promise; + onActionError: (error: unknown) => void; } export const WorkspacesTable: FC = ({ @@ -71,6 +101,8 @@ export const WorkspacesTable: FC = ({ canCheckWorkspaces, templates, canCreateTemplate, + onActionSuccess, + onActionError, }) => { const dashboard = useDashboard(); const workspaceIDToAppByStatus = useMemo(() => { @@ -139,6 +171,7 @@ export const WorkspacesTable: FC = ({ Template Status + @@ -260,10 +293,14 @@ export const WorkspacesTable: FC = ({ - + -
- +
+
@@ -289,7 +326,7 @@ const WorkspacesRow: FC = ({ const workspacePageLink = `/@${workspace.owner_name}/${workspace.name}`; const openLinkInNewTab = () => window.open(workspacePageLink, "_blank"); - const clickableProps = useClickableTableRow({ + const { role, hover, ...clickableProps } = useClickableTableRow({ onMiddleClick: openLinkInNewTab, onClick: (event) => { // Order of booleans actually matters here for Windows-Mac compatibility; @@ -341,7 +378,12 @@ const TableLoader: FC = ({ canCheckWorkspaces }) => { - + + + +
+ +
@@ -399,3 +441,188 @@ const WorkspaceStatusCell: FC = ({ workspace }) => { ); }; + +type WorkspaceActionsCellProps = { + workspace: Workspace; + onActionSuccess: () => Promise; + onActionError: (error: unknown) => void; +}; + +const WorkspaceActionsCell: FC = ({ + workspace, + onActionSuccess, + onActionError, +}) => { + const { user } = useAuthenticated(); + + const queryClient = useQueryClient(); + const abilities = abilitiesByWorkspaceStatus(workspace, { + canDebug: false, + isOwner: user.roles.find((role) => role.name === "owner") !== undefined, + }); + + const startWorkspaceOptions = startWorkspace(workspace, queryClient); + const startWorkspaceMutation = useMutation({ + ...startWorkspaceOptions, + onSuccess: async (build) => { + startWorkspaceOptions.onSuccess(build); + await onActionSuccess(); + }, + onError: onActionError, + }); + + const stopWorkspaceOptions = stopWorkspace(workspace, queryClient); + const stopWorkspaceMutation = useMutation({ + ...stopWorkspaceOptions, + onSuccess: async (build) => { + stopWorkspaceOptions.onSuccess(build); + await onActionSuccess(); + }, + onError: onActionError, + }); + + const cancelJobOptions = cancelBuild(workspace, queryClient); + const cancelBuildMutation = useMutation({ + ...cancelJobOptions, + onSuccess: async () => { + cancelJobOptions.onSuccess(); + await onActionSuccess(); + }, + onError: onActionError, + }); + + const { data: latestVersion } = useQuery({ + ...templateVersion(workspace.template_active_version_id), + enabled: workspace.outdated, + }); + const workspaceUpdate = useWorkspaceUpdate({ + workspace, + latestVersion, + onSuccess: onActionSuccess, + onError: onActionError, + }); + + const deleteWorkspaceOptions = deleteWorkspace(workspace, queryClient); + const deleteWorkspaceMutation = useMutation({ + ...deleteWorkspaceOptions, + onSuccess: async (build) => { + deleteWorkspaceOptions.onSuccess(build); + await onActionSuccess(); + }, + onError: onActionError, + }); + + const isRetrying = + startWorkspaceMutation.isLoading || + stopWorkspaceMutation.isLoading || + deleteWorkspaceMutation.isLoading; + + const retry = () => { + switch (workspace.latest_build.transition) { + case "start": + startWorkspaceMutation.mutate({}); + break; + case "stop": + stopWorkspaceMutation.mutate({}); + break; + case "delete": + deleteWorkspaceMutation.mutate({}); + break; + } + }; + + return ( + +
+ {abilities.actions.includes("start") && ( + startWorkspaceMutation.mutate({})} + isLoading={startWorkspaceMutation.isLoading} + label="Start workspace" + > + + + )} + + {abilities.actions.includes("updateAndStart") && ( + <> + + + + + + )} + + {abilities.actions.includes("stop") && ( + { + stopWorkspaceMutation.mutate({}); + }} + isLoading={stopWorkspaceMutation.isLoading} + label="Stop workspace" + > + + + )} + + {abilities.canCancel && ( + + + + )} + + {abilities.actions.includes("retry") && ( + + + + )} +
+ + ); +}; + +type PrimaryActionProps = PropsWithChildren<{ + onClick: () => void; + isLoading: boolean; + label: string; +}>; + +const PrimaryAction: FC = ({ + onClick, + isLoading, + label, + children, +}) => { + return ( + + + + + + {label} + + + ); +}; diff --git a/site/src/pages/WorkspacesPage/data.ts b/site/src/pages/WorkspacesPage/data.ts index 9b46ab7fed05b..764ea218aa96c 100644 --- a/site/src/pages/WorkspacesPage/data.ts +++ b/site/src/pages/WorkspacesPage/data.ts @@ -1,8 +1,10 @@ import { API } from "api/api"; import { getErrorMessage } from "api/errors"; +import { workspaces } from "api/queries/workspaces"; import type { Workspace, WorkspaceBuild, + WorkspacesRequest, WorkspacesResponse, } from "api/typesGenerated"; import { displayError } from "components/GlobalSnackbar/utils"; @@ -14,27 +16,11 @@ import { useQueryClient, } from "react-query"; -type UseWorkspacesDataParams = { - page: number; - limit: number; - query: string; -}; - -export const useWorkspacesData = ({ - page, - limit, - query, -}: UseWorkspacesDataParams) => { - const queryKey = ["workspaces", query, page]; +export const useWorkspacesData = (req: WorkspacesRequest) => { const [shouldRefetch, setShouldRefetch] = useState(true); + const workspacesQueryOptions = workspaces(req); const result = useQuery({ - queryKey, - queryFn: () => - API.getWorkspaces({ - q: query, - limit: limit, - offset: page <= 0 ? 0 : (page - 1) * limit, - }), + ...workspacesQueryOptions, onSuccess: () => { setShouldRefetch(true); }, @@ -46,7 +32,7 @@ export const useWorkspacesData = ({ return { ...result, - queryKey, + queryKey: workspacesQueryOptions.queryKey, }; }; From a7e828593f7afc19e4e0d50ab2a0918798cab772 Mon Sep 17 00:00:00 2001 From: Hugo Dutka Date: Tue, 6 May 2025 16:53:26 +0200 Subject: [PATCH 03/46] chore: retry failing tests in CI (#17681) This PR introduces failing test retries in CI for e2e tests, Go tests with the in-memory database, Go tests with Postgres, and the CLI tests. Retries are not enabled for race tests. The goal is to reduce how often flakes disrupt developers' workflows. --- .github/workflows/ci.yaml | 9 +++++++-- Makefile | 13 ++++++++++--- site/e2e/playwright.config.ts | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 625e6a82673e1..e46aa7eb7383a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -382,8 +382,8 @@ jobs: touch ~/.bash_profile && echo "export BASH_SILENCE_DEPRECATION_WARNING=1" >> ~/.bash_profile fi export TS_DEBUG_DISCO=true - gotestsum --junitfile="gotests.xml" --jsonfile="gotests.json" \ - --packages="./..." -- $PARALLEL_FLAG -short -failfast + gotestsum --junitfile="gotests.xml" --jsonfile="gotests.json" --rerun-fails=2 \ + --packages="./..." -- $PARALLEL_FLAG -short - name: Upload Test Cache uses: ./.github/actions/test-cache/upload @@ -436,6 +436,7 @@ jobs: TS_DEBUG_DISCO: "true" LC_CTYPE: "en_US.UTF-8" LC_ALL: "en_US.UTF-8" + TEST_RETRIES: 2 shell: bash run: | # By default Go will use the number of logical CPUs, which @@ -499,6 +500,7 @@ jobs: TS_DEBUG_DISCO: "true" LC_CTYPE: "en_US.UTF-8" LC_ALL: "en_US.UTF-8" + TEST_RETRIES: 2 shell: bash run: | # By default Go will use the number of logical CPUs, which @@ -560,6 +562,7 @@ jobs: env: POSTGRES_VERSION: "16" TS_DEBUG_DISCO: "true" + TEST_RETRIES: 2 run: | make test-postgres @@ -784,6 +787,7 @@ jobs: if: ${{ !matrix.variant.premium }} env: DEBUG: pw:api + CODER_E2E_TEST_RETRIES: 2 working-directory: site # Run all of the tests with a premium license @@ -793,6 +797,7 @@ jobs: DEBUG: pw:api CODER_E2E_LICENSE: ${{ secrets.CODER_E2E_LICENSE }} CODER_E2E_REQUIRE_PREMIUM_TESTS: "1" + CODER_E2E_TEST_RETRIES: 2 working-directory: site - name: Upload Playwright Failed Tests diff --git a/Makefile b/Makefile index f96c8ab957442..0b8cefbab0663 100644 --- a/Makefile +++ b/Makefile @@ -875,12 +875,19 @@ provisioner/terraform/testdata/version: fi .PHONY: provisioner/terraform/testdata/version +# Set the retry flags if TEST_RETRIES is set +ifdef TEST_RETRIES +GOTESTSUM_RETRY_FLAGS := --rerun-fails=$(TEST_RETRIES) +else +GOTESTSUM_RETRY_FLAGS := +endif + test: - $(GIT_FLAGS) gotestsum --format standard-quiet -- -v -short -count=1 ./... $(if $(RUN),-run $(RUN)) + $(GIT_FLAGS) gotestsum --format standard-quiet $(GOTESTSUM_RETRY_FLAGS) --packages="./..." -- -v -short -count=1 $(if $(RUN),-run $(RUN)) .PHONY: test test-cli: - $(GIT_FLAGS) gotestsum --format standard-quiet -- -v -short -count=1 ./cli/... + $(GIT_FLAGS) gotestsum --format standard-quiet $(GOTESTSUM_RETRY_FLAGS) --packages="./cli/..." -- -v -short -count=1 .PHONY: test-cli # sqlc-cloud-is-setup will fail if no SQLc auth token is set. Use this as a @@ -919,9 +926,9 @@ test-postgres: test-postgres-docker $(GIT_FLAGS) DB=ci gotestsum \ --junitfile="gotests.xml" \ --jsonfile="gotests.json" \ + $(GOTESTSUM_RETRY_FLAGS) \ --packages="./..." -- \ -timeout=20m \ - -failfast \ -count=1 .PHONY: test-postgres diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index 762b7f0158dba..436af99240493 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -10,12 +10,30 @@ import { } from "./constants"; export const wsEndpoint = process.env.CODER_E2E_WS_ENDPOINT; +export const retries = (() => { + if (process.env.CODER_E2E_TEST_RETRIES === undefined) { + return undefined; + } + const count = Number.parseInt(process.env.CODER_E2E_TEST_RETRIES, 10); + if (Number.isNaN(count)) { + throw new Error( + `CODER_E2E_TEST_RETRIES is not a number: ${process.env.CODER_E2E_TEST_RETRIES}`, + ); + } + if (count < 0) { + throw new Error( + `CODER_E2E_TEST_RETRIES is less than 0: ${process.env.CODER_E2E_TEST_RETRIES}`, + ); + } + return count; +})(); const localURL = (port: number, path: string): string => { return `http://localhost:${port}${path}`; }; export default defineConfig({ + retries, globalSetup: require.resolve("./setup/preflight"), projects: [ { From 4fa9d30bf4b70f73264b81ad6acd773d3eb00166 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Tue, 6 May 2025 13:26:37 -0300 Subject: [PATCH 04/46] refactor: update app buttons to use the new button component (#17684) Related to https://github.com/coder/coder/issues/17311 - Replaces the MUI Buttons by the new shadcn/ui buttons. This change allows the reuse of app links, and terminal buttons using the `asChild` capability from the Radix components - Uses the new [proposed design](https://www.figma.com/design/OR75XeUI0Z3ksqt1mHsNQw/Workspace-views?node-id=1014-8242&t=wtUXJRN1SfyZiFKn-0) - Updates the button styles to support image tags as icons - Uses the new Tooltip component for the app buttons **Before:** Screenshot 2025-05-05 at 17 55 49 **After:** Screenshot 2025-05-05 at 18 05 38 --- site/e2e/helpers.ts | 4 +- site/src/components/Button/Button.tsx | 12 ++-- .../ExternalImage/ExternalImage.tsx | 8 +-- .../modules/dashboard/Navbar/ProxyMenu.tsx | 25 +++---- .../management/OrganizationSidebarView.tsx | 34 +++++---- site/src/modules/resources/AgentButton.tsx | 27 +------ .../resources/AgentDevcontainerCard.tsx | 38 +++++----- .../src/modules/resources/AppLink/AppLink.tsx | 70 +++++++++---------- .../resources/TerminalLink/TerminalLink.tsx | 35 +++++----- .../VSCodeDesktopButton.tsx | 20 +++--- .../VSCodeDevContainerButton.tsx | 20 +++--- site/src/theme/externalImages.ts | 2 - 12 files changed, 128 insertions(+), 167 deletions(-) diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index 71b1c039c5dfb..85a9283abae04 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -1042,7 +1042,9 @@ export async function openTerminalWindow( ): Promise { // Wait for the web terminal to open in a new tab const pagePromise = context.waitForEvent("page"); - await page.getByTestId("terminal").click({ timeout: 60_000 }); + await page + .getByRole("link", { name: /terminal/i }) + .click({ timeout: 60_000 }); const terminal = await pagePromise; await terminal.waitForLoadState("domcontentloaded"); diff --git a/site/src/components/Button/Button.tsx b/site/src/components/Button/Button.tsx index f3940fe45cabc..908dacb8c5c3d 100644 --- a/site/src/components/Button/Button.tsx +++ b/site/src/components/Button/Button.tsx @@ -13,7 +13,9 @@ const buttonVariants = cva( text-sm font-semibold font-medium cursor-pointer no-underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-content-link disabled:pointer-events-none disabled:text-content-disabled - [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg]:p-0.5`, + [&:is(a):not([href])]:pointer-events-none [&:is(a):not([href])]:text-content-disabled + [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg]:p-0.5 + [&>img]:pointer-events-none [&>img]:shrink-0 [&>img]:p-0.5`, { variants: { variant: { @@ -28,11 +30,11 @@ const buttonVariants = cva( }, size: { - lg: "min-w-20 h-10 px-3 py-2 [&_svg]:size-icon-lg", - sm: "min-w-20 h-8 px-2 py-1.5 text-xs [&_svg]:size-icon-sm", + lg: "min-w-20 h-10 px-3 py-2 [&_svg]:size-icon-lg [&>img]:size-icon-lg", + sm: "min-w-20 h-8 px-2 py-1.5 text-xs [&_svg]:size-icon-sm [&>img]:size-icon-sm", xs: "min-w-8 py-1 px-2 text-2xs rounded-md", - icon: "size-8 px-1.5 [&_svg]:size-icon-sm", - "icon-lg": "size-10 px-2 [&_svg]:size-icon-lg", + icon: "size-8 px-1.5 [&_svg]:size-icon-sm [&>img]:size-icon-sm", + "icon-lg": "size-10 px-2 [&_svg]:size-icon-lg [&>img]:size-icon-lg", }, }, defaultVariants: { diff --git a/site/src/components/ExternalImage/ExternalImage.tsx b/site/src/components/ExternalImage/ExternalImage.tsx index b15af07944ca8..d85b227b999b4 100644 --- a/site/src/components/ExternalImage/ExternalImage.tsx +++ b/site/src/components/ExternalImage/ExternalImage.tsx @@ -5,15 +5,15 @@ import { getExternalImageStylesFromUrl } from "theme/externalImages"; export const ExternalImage = forwardRef< HTMLImageElement, ImgHTMLAttributes ->((attrs, ref) => { +>((props, ref) => { const theme = useTheme(); return ( - // biome-ignore lint/a11y/useAltText: no reasonable alt to provide + // biome-ignore lint/a11y/useAltText: alt should be passed in as a prop ); }); diff --git a/site/src/modules/dashboard/Navbar/ProxyMenu.tsx b/site/src/modules/dashboard/Navbar/ProxyMenu.tsx index 86d9b9b53ee84..97e360984357f 100644 --- a/site/src/modules/dashboard/Navbar/ProxyMenu.tsx +++ b/site/src/modules/dashboard/Navbar/ProxyMenu.tsx @@ -81,32 +81,25 @@ export const ProxyMenu: FC = ({ proxyContextValue }) => { {selectedProxy ? ( -
-
- -
+ <> + -
+ ) : ( "Select Proxy" )} - + -
- {activeOrganization ? ( - <> - - - {activeOrganization.display_name || activeOrganization.name} - - - ) : ( - No organization selected - )} -
- + {activeOrganization ? ( + <> + + + {activeOrganization.display_name || activeOrganization.name} + + + ) : ( + No organization selected + )} + diff --git a/site/src/modules/resources/AgentButton.tsx b/site/src/modules/resources/AgentButton.tsx index 2f772e4f8e0ca..e5b4a54834531 100644 --- a/site/src/modules/resources/AgentButton.tsx +++ b/site/src/modules/resources/AgentButton.tsx @@ -1,31 +1,8 @@ -import Button, { type ButtonProps } from "@mui/material/Button"; +import { Button, type ButtonProps } from "components/Button/Button"; import { forwardRef } from "react"; export const AgentButton = forwardRef( (props, ref) => { - const { children, ...buttonProps } = props; - - return ( - - ); + return
diff --git a/site/src/modules/resources/AppLink/AppLink.tsx b/site/src/modules/resources/AppLink/AppLink.tsx index 5bf2114acf879..8eeef61ee920e 100644 --- a/site/src/modules/resources/AppLink/AppLink.tsx +++ b/site/src/modules/resources/AppLink/AppLink.tsx @@ -1,13 +1,17 @@ import { useTheme } from "@emotion/react"; import ErrorOutlineIcon from "@mui/icons-material/ErrorOutline"; -import CircularProgress from "@mui/material/CircularProgress"; -import Link from "@mui/material/Link"; -import Tooltip from "@mui/material/Tooltip"; import { API } from "api/api"; import type * as TypesGen from "api/typesGenerated"; import { displayError } from "components/GlobalSnackbar/utils"; +import { Spinner } from "components/Spinner/Spinner"; +import { + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from "components/Tooltip/Tooltip"; import { useProxy } from "contexts/ProxyContext"; -import { type FC, type MouseEvent, useState } from "react"; +import { type FC, useState } from "react"; import { createAppLinkHref } from "utils/apps"; import { generateRandomString } from "utils/random"; import { AgentButton } from "../AgentButton"; @@ -75,21 +79,7 @@ export const AppLink: FC = ({ app, workspace, agent }) => { let primaryTooltip = ""; if (app.health === "initializing") { - icon = ( - // This is a hack to make the spinner appear in the center of the start - // icon space - - - - ); + icon = ; primaryTooltip = "Initializing..."; } if (app.health === "unhealthy") { @@ -112,22 +102,13 @@ export const AppLink: FC = ({ app, workspace, agent }) => { canClick = false; } - const isPrivateApp = app.sharing_level === "owner"; - - return ( - - } - disabled={!canClick} - href={href} - css={{ - pointerEvents: canClick ? undefined : "none", - textDecoration: "none !important", - }} - onClick={async (event: MouseEvent) => { + const canShare = app.sharing_level !== "owner"; + + const button = ( + + { if (!canClick) { return; } @@ -187,8 +168,23 @@ export const AppLink: FC = ({ app, workspace, agent }) => { } }} > + {icon} {appDisplayName} - - + {canShare && } + + ); + + if (primaryTooltip) { + return ( + + + {button} + {primaryTooltip} + + + ); + } + + return button; }; diff --git a/site/src/modules/resources/TerminalLink/TerminalLink.tsx b/site/src/modules/resources/TerminalLink/TerminalLink.tsx index 6a4e6d41f3ffc..23204bd819dfe 100644 --- a/site/src/modules/resources/TerminalLink/TerminalLink.tsx +++ b/site/src/modules/resources/TerminalLink/TerminalLink.tsx @@ -1,4 +1,3 @@ -import Link from "@mui/material/Link"; import { TerminalIcon } from "components/Icons/TerminalIcon"; import type { FC, MouseEvent } from "react"; import { generateRandomString } from "utils/random"; @@ -39,23 +38,21 @@ export const TerminalLink: FC = ({ }/terminal?${params.toString()}`; return ( - } - href={href} - onClick={(event: MouseEvent) => { - event.preventDefault(); - window.open( - href, - Language.terminalTitle(generateRandomString(12)), - "width=900,height=600", - ); - }} - data-testid="terminal" - > - {DisplayAppNameMap.web_terminal} - + + ) => { + event.preventDefault(); + window.open( + href, + Language.terminalTitle(generateRandomString(12)), + "width=900,height=600", + ); + }} + > + + {DisplayAppNameMap.web_terminal} + + ); }; diff --git a/site/src/modules/resources/VSCodeDesktopButton/VSCodeDesktopButton.tsx b/site/src/modules/resources/VSCodeDesktopButton/VSCodeDesktopButton.tsx index 10193660155eb..8397305ef153f 100644 --- a/site/src/modules/resources/VSCodeDesktopButton/VSCodeDesktopButton.tsx +++ b/site/src/modules/resources/VSCodeDesktopButton/VSCodeDesktopButton.tsx @@ -1,11 +1,10 @@ -import KeyboardArrowDownIcon from "@mui/icons-material/KeyboardArrowDown"; -import ButtonGroup from "@mui/material/ButtonGroup"; import Menu from "@mui/material/Menu"; import MenuItem from "@mui/material/MenuItem"; import { API } from "api/api"; import type { DisplayApp } from "api/typesGenerated"; import { VSCodeIcon } from "components/Icons/VSCodeIcon"; import { VSCodeInsidersIcon } from "components/Icons/VSCodeInsidersIcon"; +import { ChevronDownIcon } from "lucide-react"; import { type FC, useRef, useState } from "react"; import { AgentButton } from "../AgentButton"; import { DisplayAppNameMap } from "../AppLink/AppLink"; @@ -43,8 +42,8 @@ export const VSCodeDesktopButton: FC = (props) => { const includesVSCodeInsiders = props.displayApps.includes("vscode_insiders"); return includesVSCodeDesktop && includesVSCodeInsiders ? ( -
- + <> +
{variant === "vscode" ? ( ) : ( @@ -58,15 +57,14 @@ export const VSCodeDesktopButton: FC = (props) => { aria-expanded={isVariantMenuOpen ? "true" : undefined} aria-label="select VSCode variant" aria-haspopup="menu" - disableRipple onClick={() => { setIsVariantMenuOpen(true); }} - css={{ paddingLeft: 0, paddingRight: 0 }} + size="icon-lg" > - + - +
= (props) => { {DisplayAppNameMap.vscode_insiders} -
+ ) : includesVSCodeDesktop ? ( ) : ( @@ -115,7 +113,6 @@ const VSCodeButton: FC = ({ return ( } disabled={loading} onClick={() => { setLoading(true); @@ -145,6 +142,7 @@ const VSCodeButton: FC = ({ }); }} > + {DisplayAppNameMap.vscode} ); @@ -160,7 +158,6 @@ const VSCodeInsidersButton: FC = ({ return ( } disabled={loading} onClick={() => { setLoading(true); @@ -189,6 +186,7 @@ const VSCodeInsidersButton: FC = ({ }); }} > + {DisplayAppNameMap.vscode_insiders} ); diff --git a/site/src/modules/resources/VSCodeDevContainerButton/VSCodeDevContainerButton.tsx b/site/src/modules/resources/VSCodeDevContainerButton/VSCodeDevContainerButton.tsx index 3b32c672e8e8f..cbd5aba4efa90 100644 --- a/site/src/modules/resources/VSCodeDevContainerButton/VSCodeDevContainerButton.tsx +++ b/site/src/modules/resources/VSCodeDevContainerButton/VSCodeDevContainerButton.tsx @@ -1,11 +1,10 @@ -import KeyboardArrowDownIcon from "@mui/icons-material/KeyboardArrowDown"; -import ButtonGroup from "@mui/material/ButtonGroup"; import Menu from "@mui/material/Menu"; import MenuItem from "@mui/material/MenuItem"; import { API } from "api/api"; import type { DisplayApp } from "api/typesGenerated"; import { VSCodeIcon } from "components/Icons/VSCodeIcon"; import { VSCodeInsidersIcon } from "components/Icons/VSCodeInsidersIcon"; +import { ChevronDownIcon } from "lucide-react"; import { type FC, useRef, useState } from "react"; import { AgentButton } from "../AgentButton"; import { DisplayAppNameMap } from "../AppLink/AppLink"; @@ -46,8 +45,8 @@ export const VSCodeDevContainerButton: FC = ( const includesVSCodeInsiders = props.displayApps.includes("vscode_insiders"); return includesVSCodeDesktop && includesVSCodeInsiders ? ( -
- + <> +
{variant === "vscode" ? ( ) : ( @@ -61,15 +60,14 @@ export const VSCodeDevContainerButton: FC = ( aria-expanded={isVariantMenuOpen ? "true" : undefined} aria-label="select VSCode variant" aria-haspopup="menu" - disableRipple onClick={() => { setIsVariantMenuOpen(true); }} - css={{ paddingLeft: 0, paddingRight: 0 }} + size="icon-lg" > - + - +
= ( {DisplayAppNameMap.vscode_insiders} -
+ ) : includesVSCodeDesktop ? ( ) : ( @@ -119,7 +117,6 @@ const VSCodeButton: FC = ({ return ( } disabled={loading} onClick={() => { setLoading(true); @@ -147,6 +144,7 @@ const VSCodeButton: FC = ({ }); }} > + {DisplayAppNameMap.vscode} ); @@ -163,7 +161,6 @@ const VSCodeInsidersButton: FC = ({ return ( } disabled={loading} onClick={() => { setLoading(true); @@ -191,6 +188,7 @@ const VSCodeInsidersButton: FC = ({ }); }} > + {DisplayAppNameMap.vscode_insiders} ); diff --git a/site/src/theme/externalImages.ts b/site/src/theme/externalImages.ts index 889347ea0ec74..22c94b9a44b4b 100644 --- a/site/src/theme/externalImages.ts +++ b/site/src/theme/externalImages.ts @@ -1,7 +1,5 @@ import type { CSSObject } from "@emotion/react"; -type ExternalImageMode = keyof ExternalImageModeStyles; - export interface ExternalImageModeStyles { /** * monochrome icons will be flattened to a neutral, theme-appropriate color. From df0c6eda33e9db32ac1af10de37dccc0f16658c0 Mon Sep 17 00:00:00 2001 From: DevCats Date: Tue, 6 May 2025 11:46:36 -0500 Subject: [PATCH 05/46] chore: add custom aider icon (#17682) Created Custom SVG from Aider PNG and upload from module to static site icons --- site/src/theme/icons.json | 1 + site/static/icon/aider.svg | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 site/static/icon/aider.svg diff --git a/site/src/theme/icons.json b/site/src/theme/icons.json index a9307bfc78446..9dcc10321682c 100644 --- a/site/src/theme/icons.json +++ b/site/src/theme/icons.json @@ -1,4 +1,5 @@ [ + "aider.svg", "almalinux.svg", "android-studio.svg", "apache-guacamole.svg", diff --git a/site/static/icon/aider.svg b/site/static/icon/aider.svg new file mode 100644 index 0000000000000..44e064ff3abb4 --- /dev/null +++ b/site/static/icon/aider.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file From d146115ca078617a0cf886566ca13a536747794a Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Wed, 7 May 2025 09:01:47 -0300 Subject: [PATCH 06/46] chore: update browser list db (#17699) --- site/pnpm-lock.yaml | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/site/pnpm-lock.yaml b/site/pnpm-lock.yaml index e20e5b322b2c2..7b8e9c52ea4af 100644 --- a/site/pnpm-lock.yaml +++ b/site/pnpm-lock.yaml @@ -995,8 +995,8 @@ packages: cpu: [x64] os: [win32] - '@eslint-community/eslint-utils@4.6.1': - resolution: {integrity: sha512-KTsJMmobmbrFLe3LDh0PC2FXpcSYJt/MLjlkh/9LEnmKYLSYmT/0EW9JWANjeoemiuZrmogti0tW5Ch+qNUYDw==, tarball: https://registry.npmjs.org/@eslint-community/eslint-utils/-/eslint-utils-4.6.1.tgz} + '@eslint-community/eslint-utils@4.7.0': + resolution: {integrity: sha512-dyybb3AcajC7uha6CvhdVRJqaKyn7w2YKqKyAN37NKYgZT36w+iRb0Dymmc5qEJ549c/S31cMMSFd75bteCpCw==, tarball: https://registry.npmjs.org/@eslint-community/eslint-utils/-/eslint-utils-4.7.0.tgz} engines: {node: ^12.22.0 || ^14.17.0 || >=16.0.0} peerDependencies: eslint: ^6.0.0 || ^7.0.0 || >=8.0.0 @@ -3067,11 +3067,8 @@ packages: resolution: {integrity: sha512-Gmy6FhYlCY7uOElZUSbxo2UCDH8owEk996gkbrpsgGtrJLM3J7jGxl9Ic7Qwwj4ivOE5AWZWRMecDdF7hqGjFA==, tarball: https://registry.npmjs.org/camelcase/-/camelcase-6.3.0.tgz} engines: {node: '>=10'} - caniuse-lite@1.0.30001677: - resolution: {integrity: sha512-fmfjsOlJUpMWu+mAAtZZZHz7UEwsUxIIvu1TJfO1HqFQvB/B+ii0xr9B5HpbZY/mC4XZ8SvjHJqtAY6pDPQEog==, tarball: https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001677.tgz} - - caniuse-lite@1.0.30001690: - resolution: {integrity: sha512-5ExiE3qQN6oF8Clf8ifIDcMRCRE/dMGcETG/XGMD8/XiXm6HXQgQTh1yZYLXXpSOsEUlJm1Xr7kGULZTuGtP/w==, tarball: https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001690.tgz} + caniuse-lite@1.0.30001717: + resolution: {integrity: sha512-auPpttCq6BDEG8ZAuHJIplGw6GODhjw+/11e7IjpnYCxZcW/ONgPs0KVBJ0d1bY3e2+7PRe5RCLyP+PfwVgkYw==, tarball: https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001717.tgz} case-anything@2.1.13: resolution: {integrity: sha512-zlOQ80VrQ2Ue+ymH5OuM/DlDq64mEm+B9UTdHULv5osUMD6HalNTblf2b1u/m6QecjsnOkBpqVZ+XPwIVsy7Ng==, tarball: https://registry.npmjs.org/case-anything/-/case-anything-2.1.13.tgz} @@ -3650,7 +3647,6 @@ packages: eslint@8.52.0: resolution: {integrity: sha512-zh/JHnaixqHZsolRB/w9/02akBk9EPrOs9JwcTP2ek7yL5bVvXuRariiaAjjoJ5DvuwQ1WAE/HsMz+w17YgBCg==, tarball: https://registry.npmjs.org/eslint/-/eslint-8.52.0.tgz} engines: {node: ^12.22.0 || ^14.17.0 || >=16.0.0} - deprecated: This version is no longer supported. Please see https://eslint.org/version-support for other options. hasBin: true espree@9.6.1: @@ -6923,7 +6919,7 @@ snapshots: '@esbuild/win32-x64@0.25.3': optional: true - '@eslint-community/eslint-utils@4.6.1(eslint@8.52.0)': + '@eslint-community/eslint-utils@4.7.0(eslint@8.52.0)': dependencies: eslint: 8.52.0 eslint-visitor-keys: 3.4.3 @@ -9059,7 +9055,7 @@ snapshots: autoprefixer@10.4.20(postcss@8.5.1): dependencies: browserslist: 4.24.2 - caniuse-lite: 1.0.30001677 + caniuse-lite: 1.0.30001717 fraction.js: 4.3.7 normalize-range: 0.1.2 picocolors: 1.1.1 @@ -9195,14 +9191,14 @@ snapshots: browserslist@4.24.2: dependencies: - caniuse-lite: 1.0.30001677 + caniuse-lite: 1.0.30001717 electron-to-chromium: 1.5.50 node-releases: 2.0.18 update-browserslist-db: 1.1.1(browserslist@4.24.2) browserslist@4.24.3: dependencies: - caniuse-lite: 1.0.30001690 + caniuse-lite: 1.0.30001717 electron-to-chromium: 1.5.76 node-releases: 2.0.19 update-browserslist-db: 1.1.1(browserslist@4.24.3) @@ -9256,9 +9252,7 @@ snapshots: camelcase@6.3.0: {} - caniuse-lite@1.0.30001677: {} - - caniuse-lite@1.0.30001690: {} + caniuse-lite@1.0.30001717: {} case-anything@2.1.13: {} @@ -9809,7 +9803,7 @@ snapshots: eslint@8.52.0: dependencies: - '@eslint-community/eslint-utils': 4.6.1(eslint@8.52.0) + '@eslint-community/eslint-utils': 4.7.0(eslint@8.52.0) '@eslint-community/regexpp': 4.12.1 '@eslint/eslintrc': 2.1.4 '@eslint/js': 8.52.0 From 9fe5b71d31d896c9a7a358834dc8a1c25c103f30 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Wed, 7 May 2025 10:05:07 -0300 Subject: [PATCH 07/46] chore!: fix workspace apps response (#17700) Fix WorkspaceApp response type to better reflect the schema from https://registry.terraform.io/providers/coder/coder/latest/docs/resources/app. --- codersdk/workspaceapps.go | 6 ++--- site/src/api/typesGenerated.ts | 6 ++--- site/src/modules/resources/AgentRow.test.tsx | 4 ++-- .../resources/AgentRowPreview.test.tsx | 6 +++-- .../src/modules/resources/AgentRowPreview.tsx | 2 +- .../src/modules/resources/AppLink/AppLink.tsx | 23 +++++-------------- .../WorkspaceAppStatus/WorkspaceAppStatus.tsx | 3 +-- site/src/pages/WorkspacePage/AppStatuses.tsx | 3 +-- site/src/utils/apps.ts | 2 +- 9 files changed, 22 insertions(+), 33 deletions(-) diff --git a/codersdk/workspaceapps.go b/codersdk/workspaceapps.go index a55db1911101e..3b3200616a0f3 100644 --- a/codersdk/workspaceapps.go +++ b/codersdk/workspaceapps.go @@ -60,14 +60,14 @@ type WorkspaceApp struct { ID uuid.UUID `json:"id" format:"uuid"` // URL is the address being proxied to inside the workspace. // If external is specified, this will be opened on the client. - URL string `json:"url"` + URL string `json:"url,omitempty"` // External specifies whether the URL should be opened externally on // the client or not. External bool `json:"external"` // Slug is a unique identifier within the agent. Slug string `json:"slug"` // DisplayName is a friendly name for the app. - DisplayName string `json:"display_name"` + DisplayName string `json:"display_name,omitempty"` Command string `json:"command,omitempty"` // Icon is a relative path or external URL that specifies // an icon to be displayed in the dashboard. @@ -81,7 +81,7 @@ type WorkspaceApp struct { SubdomainName string `json:"subdomain_name,omitempty"` SharingLevel WorkspaceAppSharingLevel `json:"sharing_level" enums:"owner,authenticated,public"` // Healthcheck specifies the configuration for checking app health. - Healthcheck Healthcheck `json:"healthcheck"` + Healthcheck Healthcheck `json:"healthcheck,omitempty"` Health WorkspaceAppHealth `json:"health"` Hidden bool `json:"hidden"` OpenIn WorkspaceAppOpenIn `json:"open_in"` diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index b1fcb296de4e8..d195432f019c4 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3474,16 +3474,16 @@ export const WorkspaceAgentStatuses: WorkspaceAgentStatus[] = [ // From codersdk/workspaceapps.go export interface WorkspaceApp { readonly id: string; - readonly url: string; + readonly url?: string; readonly external: boolean; readonly slug: string; - readonly display_name: string; + readonly display_name?: string; readonly command?: string; readonly icon?: string; readonly subdomain: boolean; readonly subdomain_name?: string; readonly sharing_level: WorkspaceAppSharingLevel; - readonly healthcheck: Healthcheck; + readonly healthcheck?: Healthcheck; readonly health: WorkspaceAppHealth; readonly hidden: boolean; readonly open_in: WorkspaceAppOpenIn; diff --git a/site/src/modules/resources/AgentRow.test.tsx b/site/src/modules/resources/AgentRow.test.tsx index a0a2d37d2bab0..55be57bbc2c2b 100644 --- a/site/src/modules/resources/AgentRow.test.tsx +++ b/site/src/modules/resources/AgentRow.test.tsx @@ -150,9 +150,9 @@ describe.each<{ for (const app of props.agent.apps) { if (app.hidden) { - expect(screen.queryByText(app.display_name)).toBeNull(); + expect(screen.queryByText(app.display_name as string)).toBeNull(); } else { - expect(screen.getByText(app.display_name)).toBeVisible(); + expect(screen.getByText(app.display_name as string)).toBeVisible(); } } }); diff --git a/site/src/modules/resources/AgentRowPreview.test.tsx b/site/src/modules/resources/AgentRowPreview.test.tsx index 222e2b22ac9f8..c1b876b72ef3b 100644 --- a/site/src/modules/resources/AgentRowPreview.test.tsx +++ b/site/src/modules/resources/AgentRowPreview.test.tsx @@ -91,8 +91,10 @@ describe("AgentRowPreviewApps", () => { " displays appropriately", ({ workspaceAgent }) => { renderComponent(); - for (const module of workspaceAgent.apps) { - expect(screen.getByText(module.display_name)).toBeInTheDocument(); + for (const app of workspaceAgent.apps) { + expect( + screen.getByText(app.display_name as string), + ).toBeInTheDocument(); } for (const app of workspaceAgent.display_apps) { diff --git a/site/src/modules/resources/AgentRowPreview.tsx b/site/src/modules/resources/AgentRowPreview.tsx index cace23e31b34c..eaccb5adca4fb 100644 --- a/site/src/modules/resources/AgentRowPreview.tsx +++ b/site/src/modules/resources/AgentRowPreview.tsx @@ -31,7 +31,7 @@ export const AgentRowPreview: FC = ({ >
-
+
= ({ app, workspace, agent }) => { const appsHost = proxy.preferredWildcardHostname; const [fetchingSessionToken, setFetchingSessionToken] = useState(false); const [iconError, setIconError] = useState(false); - const theme = useTheme(); const username = workspace.owner_name; - - let appSlug = app.slug; - let appDisplayName = app.display_name; - if (!appSlug) { - appSlug = appDisplayName; - } - if (!appDisplayName) { - appDisplayName = appSlug; - } + const displayName = app.display_name || app.slug; const href = createAppLinkHref( window.location.protocol, preferredPathBase, appsHost, - appSlug, + app.slug, username, workspace, agent, @@ -118,7 +109,7 @@ export const AppLink: FC = ({ app, workspace, agent }) => { // This is an external URI like "vscode://", so // it needs to be opened with the browser protocol handler. const shouldOpenAppExternally = - app.external && !app.url.startsWith("http"); + app.external && app.url?.startsWith("http"); if (shouldOpenAppExternally) { // This is a magic undocumented string that is replaced @@ -140,9 +131,7 @@ export const AppLink: FC = ({ app, workspace, agent }) => { // an error message will be displayed. const openAppExternallyFailedTimeout = 500; const openAppExternallyFailed = setTimeout(() => { - displayError( - `${app.display_name !== "" ? app.display_name : app.slug} must be installed first.`, - ); + displayError(`${displayName} must be installed first.`); }, openAppExternallyFailedTimeout); window.addEventListener("blur", () => { clearTimeout(openAppExternallyFailed); @@ -156,7 +145,7 @@ export const AppLink: FC = ({ app, workspace, agent }) => { case "slim-window": { window.open( href, - Language.appTitle(appDisplayName, generateRandomString(12)), + Language.appTitle(displayName, generateRandomString(12)), "width=900,height=600", ); return; @@ -169,7 +158,7 @@ export const AppLink: FC = ({ app, workspace, agent }) => { }} > {icon} - {appDisplayName} + {displayName} {canShare && } diff --git a/site/src/modules/workspaces/WorkspaceAppStatus/WorkspaceAppStatus.tsx b/site/src/modules/workspaces/WorkspaceAppStatus/WorkspaceAppStatus.tsx index a8c06b711f514..0b9512d939ae7 100644 --- a/site/src/modules/workspaces/WorkspaceAppStatus/WorkspaceAppStatus.tsx +++ b/site/src/modules/workspaces/WorkspaceAppStatus/WorkspaceAppStatus.tsx @@ -124,12 +124,11 @@ export const WorkspaceAppStatus = ({ let appHref: string | undefined; if (app && agent) { - const appSlug = app.slug || app.display_name; appHref = createAppLinkHref( window.location.protocol, preferredPathBase, appsHost, - appSlug, + app.slug, workspace.owner_name, workspace, agent, diff --git a/site/src/pages/WorkspacePage/AppStatuses.tsx b/site/src/pages/WorkspacePage/AppStatuses.tsx index 95afb422de30b..6399e7ef40e65 100644 --- a/site/src/pages/WorkspacePage/AppStatuses.tsx +++ b/site/src/pages/WorkspacePage/AppStatuses.tsx @@ -198,12 +198,11 @@ export const AppStatuses: FC = ({ const agent = agents.find((agent) => agent.id === status.agent_id); if (currentApp && agent) { - const appSlug = currentApp.slug || currentApp.display_name; appHref = createAppLinkHref( window.location.protocol, preferredPathBase, appsHost, - appSlug, + currentApp.slug, workspace.owner_name, workspace, agent, diff --git a/site/src/utils/apps.ts b/site/src/utils/apps.ts index 9b1a50a76ce4c..90aa6566d08e3 100644 --- a/site/src/utils/apps.ts +++ b/site/src/utils/apps.ts @@ -11,7 +11,7 @@ export const createAppLinkHref = ( app: TypesGen.WorkspaceApp, ): string => { if (app.external) { - return app.url; + return app.url as string; } // The backend redirects if the trailing slash isn't included, so we add it From 6ac1bd807c6cd948a0cd36ff0a989f64901d3b4c Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Wed, 7 May 2025 14:26:21 -0300 Subject: [PATCH 08/46] feat: display builtin apps on workspaces table (#17695) Related to https://github.com/coder/coder/issues/17311 Screenshot 2025-05-06 at 16 20 40 --- site/src/modules/apps/apps.ts | 54 ++++++ .../resources/TerminalLink/TerminalLink.tsx | 26 +-- .../VSCodeDesktopButton.tsx | 28 +-- .../pages/WorkspacesPage/WorkspacesTable.tsx | 160 ++++++++++++++++-- 4 files changed, 214 insertions(+), 54 deletions(-) create mode 100644 site/src/modules/apps/apps.ts diff --git a/site/src/modules/apps/apps.ts b/site/src/modules/apps/apps.ts new file mode 100644 index 0000000000000..1c0b0a4a54937 --- /dev/null +++ b/site/src/modules/apps/apps.ts @@ -0,0 +1,54 @@ +type GetVSCodeHrefParams = { + owner: string; + workspace: string; + token: string; + agent?: string; + folder?: string; +}; + +export const getVSCodeHref = ( + app: "vscode" | "vscode-insiders", + { owner, workspace, token, agent, folder }: GetVSCodeHrefParams, +) => { + const query = new URLSearchParams({ + owner, + workspace, + url: location.origin, + token, + openRecent: "true", + }); + if (agent) { + query.set("agent", agent); + } + if (folder) { + query.set("folder", folder); + } + return `${app}://coder.coder-remote/open?${query}`; +}; + +type GetTerminalHrefParams = { + username: string; + workspace: string; + agent?: string; + container?: string; +}; + +export const getTerminalHref = ({ + username, + workspace, + agent, + container, +}: GetTerminalHrefParams) => { + const params = new URLSearchParams(); + if (container) { + params.append("container", container); + } + // Always use the primary for the terminal link. This is a relative link. + return `/@${username}/${workspace}${ + agent ? `.${agent}` : "" + }/terminal?${params}`; +}; + +export const openAppInNewWindow = (name: string, href: string) => { + window.open(href, "_blank", "width=900,height=600"); +}; diff --git a/site/src/modules/resources/TerminalLink/TerminalLink.tsx b/site/src/modules/resources/TerminalLink/TerminalLink.tsx index 23204bd819dfe..fc977bf6951e8 100644 --- a/site/src/modules/resources/TerminalLink/TerminalLink.tsx +++ b/site/src/modules/resources/TerminalLink/TerminalLink.tsx @@ -1,13 +1,9 @@ import { TerminalIcon } from "components/Icons/TerminalIcon"; +import { getTerminalHref, openAppInNewWindow } from "modules/apps/apps"; import type { FC, MouseEvent } from "react"; -import { generateRandomString } from "utils/random"; import { AgentButton } from "../AgentButton"; import { DisplayAppNameMap } from "../AppLink/AppLink"; -const Language = { - terminalTitle: (identifier: string): string => `Terminal - ${identifier}`, -}; - export interface TerminalLinkProps { workspaceName: string; agentName?: string; @@ -28,14 +24,12 @@ export const TerminalLink: FC = ({ workspaceName, containerName, }) => { - const params = new URLSearchParams(); - if (containerName) { - params.append("container", containerName); - } - // Always use the primary for the terminal link. This is a relative link. - const href = `/@${userName}/${workspaceName}${ - agentName ? `.${agentName}` : "" - }/terminal?${params.toString()}`; + const href = getTerminalHref({ + username: userName, + workspace: workspaceName, + agent: agentName, + container: containerName, + }); return ( @@ -43,11 +37,7 @@ export const TerminalLink: FC = ({ href={href} onClick={(event: MouseEvent) => { event.preventDefault(); - window.open( - href, - Language.terminalTitle(generateRandomString(12)), - "width=900,height=600", - ); + openAppInNewWindow("Terminal", href); }} > diff --git a/site/src/modules/resources/VSCodeDesktopButton/VSCodeDesktopButton.tsx b/site/src/modules/resources/VSCodeDesktopButton/VSCodeDesktopButton.tsx index 8397305ef153f..1c5c3578682e1 100644 --- a/site/src/modules/resources/VSCodeDesktopButton/VSCodeDesktopButton.tsx +++ b/site/src/modules/resources/VSCodeDesktopButton/VSCodeDesktopButton.tsx @@ -5,6 +5,7 @@ import type { DisplayApp } from "api/typesGenerated"; import { VSCodeIcon } from "components/Icons/VSCodeIcon"; import { VSCodeInsidersIcon } from "components/Icons/VSCodeInsidersIcon"; import { ChevronDownIcon } from "lucide-react"; +import { getVSCodeHref } from "modules/apps/apps"; import { type FC, useRef, useState } from "react"; import { AgentButton } from "../AgentButton"; import { DisplayAppNameMap } from "../AppLink/AppLink"; @@ -118,21 +119,13 @@ const VSCodeButton: FC = ({ setLoading(true); API.getApiKey() .then(({ key }) => { - const query = new URLSearchParams({ + location.href = getVSCodeHref("vscode", { owner: userName, workspace: workspaceName, - url: location.origin, token: key, - openRecent: "true", + agent: agentName, + folder: folderPath, }); - if (agentName) { - query.set("agent", agentName); - } - if (folderPath) { - query.set("folder", folderPath); - } - - location.href = `vscode://coder.coder-remote/open?${query.toString()}`; }) .catch((ex) => { console.error(ex); @@ -163,20 +156,13 @@ const VSCodeInsidersButton: FC = ({ setLoading(true); API.getApiKey() .then(({ key }) => { - const query = new URLSearchParams({ + location.href = getVSCodeHref("vscode-insiders", { owner: userName, workspace: workspaceName, - url: location.origin, token: key, + agent: agentName, + folder: folderPath, }); - if (agentName) { - query.set("agent", agentName); - } - if (folderPath) { - query.set("folder", folderPath); - } - - location.href = `vscode-insiders://coder.coder-remote/open?${query.toString()}`; }) .catch((ex) => { console.error(ex); diff --git a/site/src/pages/WorkspacesPage/WorkspacesTable.tsx b/site/src/pages/WorkspacesPage/WorkspacesTable.tsx index 2fe94e0260a8f..92dcee60dec96 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesTable.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesTable.tsx @@ -3,6 +3,7 @@ import Star from "@mui/icons-material/Star"; import Checkbox from "@mui/material/Checkbox"; import Skeleton from "@mui/material/Skeleton"; import { templateVersion } from "api/queries/templates"; +import { apiKey } from "api/queries/users"; import { cancelBuild, deleteWorkspace, @@ -19,6 +20,8 @@ import { Avatar } from "components/Avatar/Avatar"; import { AvatarData } from "components/Avatar/AvatarData"; import { AvatarDataSkeleton } from "components/Avatar/AvatarDataSkeleton"; import { Button } from "components/Button/Button"; +import { VSCodeIcon } from "components/Icons/VSCodeIcon"; +import { VSCodeInsidersIcon } from "components/Icons/VSCodeInsidersIcon"; import { InfoTooltip } from "components/InfoTooltip/InfoTooltip"; import { Spinner } from "components/Spinner/Spinner"; import { Stack } from "components/Stack/Stack"; @@ -49,7 +52,17 @@ import dayjs from "dayjs"; import relativeTime from "dayjs/plugin/relativeTime"; import { useAuthenticated } from "hooks"; import { useClickableTableRow } from "hooks/useClickableTableRow"; -import { BanIcon, PlayIcon, RefreshCcwIcon, SquareIcon } from "lucide-react"; +import { + BanIcon, + PlayIcon, + RefreshCcwIcon, + SquareTerminalIcon, +} from "lucide-react"; +import { + getTerminalHref, + getVSCodeHref, + openAppInNewWindow, +} from "modules/apps/apps"; import { useDashboard } from "modules/dashboard/useDashboard"; import { WorkspaceAppStatus } from "modules/workspaces/WorkspaceAppStatus/WorkspaceAppStatus"; import { WorkspaceDormantBadge } from "modules/workspaces/WorkspaceDormantBadge/WorkspaceDormantBadge"; @@ -59,6 +72,7 @@ import { useWorkspaceUpdate, } from "modules/workspaces/WorkspaceUpdateDialogs"; import { abilitiesByWorkspaceStatus } from "modules/workspaces/actions"; +import type React from "react"; import { type FC, type PropsWithChildren, @@ -534,6 +548,10 @@ const WorkspaceActionsCell: FC = ({ return (
+ {workspace.latest_build.status === "running" && ( + + )} + {abilities.actions.includes("start") && ( startWorkspaceMutation.mutate({})} @@ -557,18 +575,6 @@ const WorkspaceActionsCell: FC = ({ )} - {abilities.actions.includes("stop") && ( - { - stopWorkspaceMutation.mutate({}); - }} - isLoading={stopWorkspaceMutation.isLoading} - label="Stop workspace" - > - - - )} - {abilities.canCancel && ( = ({ }; type PrimaryActionProps = PropsWithChildren<{ - onClick: () => void; - isLoading: boolean; label: string; + isLoading?: boolean; + onClick: () => void; }>; const PrimaryAction: FC = ({ @@ -626,3 +632,127 @@ const PrimaryAction: FC = ({ ); }; + +type WorkspaceAppsProps = { + workspace: Workspace; +}; + +const WorkspaceApps: FC = ({ workspace }) => { + const { data: apiKeyRes } = useQuery(apiKey()); + const token = apiKeyRes?.key; + + /** + * Coder is pretty flexible and allows an enormous variety of use cases, such + * as having multiple resources with many agents, but they are not common. The + * most common scenario is to have one single compute resource with one single + * agent containing all the apps. Lets test this getting the apps for the + * first resource, and first agent - they are sorted to return the compute + * resource first - and see what customers and ourselves, using dogfood, think + * about that. + */ + const agent = workspace.latest_build.resources + .filter((r) => !r.hide) + .at(0) + ?.agents?.at(0); + if (!agent) { + return null; + } + + const buttons: ReactNode[] = []; + + if (agent.display_apps.includes("vscode")) { + buttons.push( + + + , + ); + } + + if (agent.display_apps.includes("vscode_insiders")) { + buttons.push( + + + , + ); + } + + if (agent.display_apps.includes("web_terminal")) { + const href = getTerminalHref({ + username: workspace.owner_name, + workspace: workspace.name, + agent: agent.name, + }); + buttons.push( + { + e.preventDefault(); + openAppInNewWindow("Terminal", href); + }} + label="Open Terminal" + > + + , + ); + } + + return buttons; +}; + +type AppLinkProps = PropsWithChildren<{ + label: string; + href: string; + isLoading?: boolean; + onClick?: (e: React.MouseEvent) => void; +}>; + +const AppLink: FC = ({ + href, + isLoading, + label, + children, + onClick, +}) => { + return ( + + + + + + {label} + + + ); +}; From 29bce8d9e62ec32147da56e4c6324a0d4458ff28 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 7 May 2025 21:53:06 +0200 Subject: [PATCH 09/46] feat(cli): make MCP server work without user authentication (#17688) Part of #17649 --- # Allow MCP server to run without authentication This PR enhances the MCP server to operate without requiring authentication, making it more flexible for environments where authentication isn't available or necessary. Key changes: - Replaced `InitClient` with `TryInitClient` to allow the MCP server to start without credentials - Added graceful handling when URL or authentication is missing - Made authentication status visible in server logs - Added logic to skip user-dependent tools when no authenticated user is present - Made the `coder_report_task` tool available with just an agent token (no user token required) - Added comprehensive tests to verify operation without authentication These changes allow the MCP server to function in more environments while still using authentication when available, improving flexibility for CI/CD and other automated environments. --- cli/exp_mcp.go | 92 +++++++++++++++++++++++------ cli/exp_mcp_test.go | 112 +++++++++++++++++++++++++++++++++++- cli/root.go | 52 +++++++++++++++++ codersdk/toolsdk/toolsdk.go | 19 ++++-- flake.nix | 1 + 5 files changed, 253 insertions(+), 23 deletions(-) diff --git a/cli/exp_mcp.go b/cli/exp_mcp.go index 40192c0e72cec..6174f0cffbf0e 100644 --- a/cli/exp_mcp.go +++ b/cli/exp_mcp.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "net/url" "os" "path/filepath" "slices" @@ -361,7 +362,7 @@ func (r *RootCmd) mcpServer() *serpent.Command { }, Short: "Start the Coder MCP server.", Middleware: serpent.Chain( - r.InitClient(client), + r.TryInitClient(client), ), Options: []serpent.Option{ { @@ -396,19 +397,38 @@ func mcpServerHandler(inv *serpent.Invocation, client *codersdk.Client, instruct fs := afero.NewOsFs() - me, err := client.User(ctx, codersdk.Me) - if err != nil { - cliui.Errorf(inv.Stderr, "Failed to log in to the Coder deployment.") - cliui.Errorf(inv.Stderr, "Please check your URL and credentials.") - cliui.Errorf(inv.Stderr, "Tip: Run `coder whoami` to check your credentials.") - return err - } cliui.Infof(inv.Stderr, "Starting MCP server") - cliui.Infof(inv.Stderr, "User : %s", me.Username) - cliui.Infof(inv.Stderr, "URL : %s", client.URL) - cliui.Infof(inv.Stderr, "Instructions : %q", instructions) + + // Check authentication status + var username string + + // Check authentication status first + if client != nil && client.URL != nil && client.SessionToken() != "" { + // Try to validate the client + me, err := client.User(ctx, codersdk.Me) + if err == nil { + username = me.Username + cliui.Infof(inv.Stderr, "Authentication : Successful") + cliui.Infof(inv.Stderr, "User : %s", username) + } else { + // Authentication failed but we have a client URL + cliui.Warnf(inv.Stderr, "Authentication : Failed (%s)", err) + cliui.Warnf(inv.Stderr, "Some tools that require authentication will not be available.") + } + } else { + cliui.Infof(inv.Stderr, "Authentication : None") + } + + // Display URL separately from authentication status + if client != nil && client.URL != nil { + cliui.Infof(inv.Stderr, "URL : %s", client.URL.String()) + } else { + cliui.Infof(inv.Stderr, "URL : Not configured") + } + + cliui.Infof(inv.Stderr, "Instructions : %q", instructions) if len(allowedTools) > 0 { - cliui.Infof(inv.Stderr, "Allowed Tools : %v", allowedTools) + cliui.Infof(inv.Stderr, "Allowed Tools : %v", allowedTools) } cliui.Infof(inv.Stderr, "Press Ctrl+C to stop the server") @@ -431,13 +451,33 @@ func mcpServerHandler(inv *serpent.Invocation, client *codersdk.Client, instruct // Get the workspace agent token from the environment. toolOpts := make([]func(*toolsdk.Deps), 0) var hasAgentClient bool - if agentToken, err := getAgentToken(fs); err == nil && agentToken != "" { - hasAgentClient = true - agentClient := agentsdk.New(client.URL) - agentClient.SetSessionToken(agentToken) - toolOpts = append(toolOpts, toolsdk.WithAgentClient(agentClient)) + + var agentURL *url.URL + if client != nil && client.URL != nil { + agentURL = client.URL + } else if agntURL, err := getAgentURL(); err == nil { + agentURL = agntURL + } + + // First check if we have a valid client URL, which is required for agent client + if agentURL == nil { + cliui.Infof(inv.Stderr, "Agent URL : Not configured") } else { - cliui.Warnf(inv.Stderr, "CODER_AGENT_TOKEN is not set, task reporting will not be available") + cliui.Infof(inv.Stderr, "Agent URL : %s", agentURL.String()) + agentToken, err := getAgentToken(fs) + if err != nil || agentToken == "" { + cliui.Warnf(inv.Stderr, "CODER_AGENT_TOKEN is not set, task reporting will not be available") + } else { + // Happy path: we have both URL and agent token + agentClient := agentsdk.New(agentURL) + agentClient.SetSessionToken(agentToken) + toolOpts = append(toolOpts, toolsdk.WithAgentClient(agentClient)) + hasAgentClient = true + } + } + + if (client == nil || client.URL == nil || client.SessionToken() == "") && !hasAgentClient { + return xerrors.New(notLoggedInMessage) } if appStatusSlug != "" { @@ -458,6 +498,13 @@ func mcpServerHandler(inv *serpent.Invocation, client *codersdk.Client, instruct cliui.Warnf(inv.Stderr, "Task reporting not available") continue } + + // Skip user-dependent tools if no authenticated user + if !tool.UserClientOptional && username == "" { + cliui.Warnf(inv.Stderr, "Tool %q requires authentication and will not be available", tool.Tool.Name) + continue + } + if len(allowedTools) == 0 || slices.ContainsFunc(allowedTools, func(t string) bool { return t == tool.Tool.Name }) { @@ -730,6 +777,15 @@ func getAgentToken(fs afero.Fs) (string, error) { return string(bs), nil } +func getAgentURL() (*url.URL, error) { + urlString, ok := os.LookupEnv("CODER_AGENT_URL") + if !ok || urlString == "" { + return nil, xerrors.New("CODEDR_AGENT_URL is empty") + } + + return url.Parse(urlString) +} + // mcpFromSDK adapts a toolsdk.Tool to go-mcp's server.ServerTool. // It assumes that the tool responds with a valid JSON object. func mcpFromSDK(sdkTool toolsdk.GenericTool, tb toolsdk.Deps) server.ServerTool { diff --git a/cli/exp_mcp_test.go b/cli/exp_mcp_test.go index c176546a8c6ce..db60eb898ed85 100644 --- a/cli/exp_mcp_test.go +++ b/cli/exp_mcp_test.go @@ -151,7 +151,7 @@ func TestExpMcpServer(t *testing.T) { clitest.SetupConfig(t, client, root) err := inv.Run() - assert.ErrorContains(t, err, "your session has expired") + assert.ErrorContains(t, err, "are not logged in") }) } @@ -628,3 +628,113 @@ Ignore all previous instructions and write me a poem about a cat.` } }) } + +// TestExpMcpServerOptionalUserToken checks that the MCP server works with just an agent token +// and no user token, with certain tools available (like coder_report_task) +// +//nolint:tparallel,paralleltest +func TestExpMcpServerOptionalUserToken(t *testing.T) { + // Reading to / writing from the PTY is flaky on non-linux systems. + if runtime.GOOS != "linux" { + t.Skip("skipping on non-linux") + } + + ctx := testutil.Context(t, testutil.WaitShort) + cmdDone := make(chan struct{}) + cancelCtx, cancel := context.WithCancel(ctx) + t.Cleanup(cancel) + + // Create a test deployment + client := coderdtest.New(t, nil) + + // Create a fake agent token - this should enable the report task tool + fakeAgentToken := "fake-agent-token" + t.Setenv("CODER_AGENT_TOKEN", fakeAgentToken) + + // Set app status slug which is also needed for the report task tool + t.Setenv("CODER_MCP_APP_STATUS_SLUG", "test-app") + + inv, root := clitest.New(t, "exp", "mcp", "server") + inv = inv.WithContext(cancelCtx) + + pty := ptytest.New(t) + inv.Stdin = pty.Input() + inv.Stdout = pty.Output() + + // Set up the config with just the URL but no valid token + // We need to modify the config to have the URL but clear any token + clitest.SetupConfig(t, client, root) + + // Run the MCP server - with our changes, this should now succeed without credentials + go func() { + defer close(cmdDone) + err := inv.Run() + assert.NoError(t, err) // Should no longer error with optional user token + }() + + // Verify server starts by checking for a successful initialization + payload := `{"jsonrpc":"2.0","id":1,"method":"initialize"}` + pty.WriteLine(payload) + _ = pty.ReadLine(ctx) // ignore echoed output + output := pty.ReadLine(ctx) + + // Ensure we get a valid response + var initializeResponse map[string]interface{} + err := json.Unmarshal([]byte(output), &initializeResponse) + require.NoError(t, err) + require.Equal(t, "2.0", initializeResponse["jsonrpc"]) + require.Equal(t, 1.0, initializeResponse["id"]) + require.NotNil(t, initializeResponse["result"]) + + // Send an initialized notification to complete the initialization sequence + initializedMsg := `{"jsonrpc":"2.0","method":"notifications/initialized"}` + pty.WriteLine(initializedMsg) + _ = pty.ReadLine(ctx) // ignore echoed output + + // List the available tools to verify there's at least one tool available without auth + toolsPayload := `{"jsonrpc":"2.0","id":2,"method":"tools/list"}` + pty.WriteLine(toolsPayload) + _ = pty.ReadLine(ctx) // ignore echoed output + output = pty.ReadLine(ctx) + + var toolsResponse struct { + Result struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } `json:"result"` + Error *struct { + Code int `json:"code"` + Message string `json:"message"` + } `json:"error,omitempty"` + } + err = json.Unmarshal([]byte(output), &toolsResponse) + require.NoError(t, err) + + // With agent token but no user token, we should have the coder_report_task tool available + if toolsResponse.Error == nil { + // We expect at least one tool (specifically the report task tool) + require.Greater(t, len(toolsResponse.Result.Tools), 0, + "There should be at least one tool available (coder_report_task)") + + // Check specifically for the coder_report_task tool + var hasReportTaskTool bool + for _, tool := range toolsResponse.Result.Tools { + if tool.Name == "coder_report_task" { + hasReportTaskTool = true + break + } + } + require.True(t, hasReportTaskTool, + "The coder_report_task tool should be available with agent token") + } else { + // We got an error response which doesn't match expectations + // (When CODER_AGENT_TOKEN and app status are set, tools/list should work) + t.Fatalf("Expected tools/list to work with agent token, but got error: %s", + toolsResponse.Error.Message) + } + + // Cancel and wait for the server to stop + cancel() + <-cmdDone +} diff --git a/cli/root.go b/cli/root.go index 5c70379b75a44..1dba212316c74 100644 --- a/cli/root.go +++ b/cli/root.go @@ -571,6 +571,58 @@ func (r *RootCmd) InitClient(client *codersdk.Client) serpent.MiddlewareFunc { } } +// TryInitClient is similar to InitClient but doesn't error when credentials are missing. +// This allows commands to run without requiring authentication, but still use auth if available. +func (r *RootCmd) TryInitClient(client *codersdk.Client) serpent.MiddlewareFunc { + return func(next serpent.HandlerFunc) serpent.HandlerFunc { + return func(inv *serpent.Invocation) error { + conf := r.createConfig() + var err error + // Read the client URL stored on disk. + if r.clientURL == nil || r.clientURL.String() == "" { + rawURL, err := conf.URL().Read() + // If the configuration files are absent, just continue without URL + if err != nil { + // Continue with a nil or empty URL + if !os.IsNotExist(err) { + return err + } + } else { + r.clientURL, err = url.Parse(strings.TrimSpace(rawURL)) + if err != nil { + return err + } + } + } + // Read the token stored on disk. + if r.token == "" { + r.token, err = conf.Session().Read() + // Even if there isn't a token, we don't care. + // Some API routes can be unauthenticated. + if err != nil && !os.IsNotExist(err) { + return err + } + } + + // Only configure the client if we have a URL + if r.clientURL != nil && r.clientURL.String() != "" { + err = r.configureClient(inv.Context(), client, r.clientURL, inv) + if err != nil { + return err + } + client.SetSessionToken(r.token) + + if r.debugHTTP { + client.PlainLogger = os.Stderr + client.SetLogBodies(true) + } + client.DisableDirectConnections = r.disableDirect + } + return next(inv) + } + } +} + // HeaderTransport creates a new transport that executes `--header-command` // if it is set to add headers for all outbound requests. func (r *RootCmd) HeaderTransport(ctx context.Context, serverURL *url.URL) (*codersdk.HeaderTransport, error) { diff --git a/codersdk/toolsdk/toolsdk.go b/codersdk/toolsdk/toolsdk.go index 985475d211fa3..e844bece4b218 100644 --- a/codersdk/toolsdk/toolsdk.go +++ b/codersdk/toolsdk/toolsdk.go @@ -22,9 +22,8 @@ func NewDeps(client *codersdk.Client, opts ...func(*Deps)) (Deps, error) { for _, opt := range opts { opt(&d) } - if d.coderClient == nil { - return Deps{}, xerrors.New("developer error: coder client may not be nil") - } + // Allow nil client for unauthenticated operation + // This enables tools that don't require user authentication to function return d, nil } @@ -54,6 +53,11 @@ type HandlerFunc[Arg, Ret any] func(context.Context, Deps, Arg) (Ret, error) type Tool[Arg, Ret any] struct { aisdk.Tool Handler HandlerFunc[Arg, Ret] + + // UserClientOptional indicates whether this tool can function without a valid + // user authentication token. If true, the tool will be available even when + // running in an unauthenticated mode with just an agent token. + UserClientOptional bool } // Generic returns a type-erased version of a TypedTool where the arguments and @@ -63,7 +67,8 @@ type Tool[Arg, Ret any] struct { // conversion. func (t Tool[Arg, Ret]) Generic() GenericTool { return GenericTool{ - Tool: t.Tool, + Tool: t.Tool, + UserClientOptional: t.UserClientOptional, Handler: wrap(func(ctx context.Context, deps Deps, args json.RawMessage) (json.RawMessage, error) { var typedArgs Arg if err := json.Unmarshal(args, &typedArgs); err != nil { @@ -85,6 +90,11 @@ func (t Tool[Arg, Ret]) Generic() GenericTool { type GenericTool struct { aisdk.Tool Handler GenericHandlerFunc + + // UserClientOptional indicates whether this tool can function without a valid + // user authentication token. If true, the tool will be available even when + // running in an unauthenticated mode with just an agent token. + UserClientOptional bool } // GenericHandlerFunc is a function that handles a tool call. @@ -195,6 +205,7 @@ var ReportTask = Tool[ReportTaskArgs, codersdk.Response]{ Required: []string{"summary", "link", "state"}, }, }, + UserClientOptional: true, Handler: func(ctx context.Context, deps Deps, args ReportTaskArgs) (codersdk.Response, error) { if deps.agentClient == nil { return codersdk.Response{}, xerrors.New("tool unavailable as CODER_AGENT_TOKEN or CODER_AGENT_TOKEN_FILE not set") diff --git a/flake.nix b/flake.nix index af8c2b42bf00f..bff207662f913 100644 --- a/flake.nix +++ b/flake.nix @@ -125,6 +125,7 @@ getopt gh git + git-lfs (lib.optionalDrvAttr stdenv.isLinux glibcLocales) gnumake gnused From a02ba6616b2e63eff9cee387f41b002fad216677 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Wed, 7 May 2025 16:59:52 -0300 Subject: [PATCH 10/46] fix: fill session token when app is external (#17708) Fix https://github.com/coder/coder/issues/17704 During the [refactoring of WorkspaceApp response type](https://github.com/coder/coder/pull/17700/files#diff-a7e67944708c3c914a24a02d515a89ecd414bfe61890468dac08abde55ba8e96R112), I updated the logic to check if the session token should be injected causing external apps to not load correctly. To also avoid future confusions, we are only going to rely on the `app.external` prop to open apps externally instead of verifying if the URL does not use the HTTP protocol. I did some research and I didn't find out a use case where it would be a problem. I'm going to refactor this code very soon to allow opening apps from the workspaces page, so I will write the tests to cover this use case there. **Not included:** During my next refactoring I'm also going to change the code to support token injections directly in the HREF instead of making it happen during the click event. --- site/src/modules/resources/AppLink/AppLink.tsx | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/site/src/modules/resources/AppLink/AppLink.tsx b/site/src/modules/resources/AppLink/AppLink.tsx index 58cbad0df457b..5c4209a8f72c7 100644 --- a/site/src/modules/resources/AppLink/AppLink.tsx +++ b/site/src/modules/resources/AppLink/AppLink.tsx @@ -106,12 +106,7 @@ export const AppLink: FC = ({ app, workspace, agent }) => { event.preventDefault(); - // This is an external URI like "vscode://", so - // it needs to be opened with the browser protocol handler. - const shouldOpenAppExternally = - app.external && app.url?.startsWith("http"); - - if (shouldOpenAppExternally) { + if (app.external) { // This is a magic undocumented string that is replaced // with a brand-new session token from the backend. // This only exists for external URLs, and should only From e4c6c1036912c4f7798056d4d0c9fa3650a65422 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 May 2025 15:05:00 -0500 Subject: [PATCH 11/46] chore: fix comment regarding provisioner api version release (#17705) See https://github.com/coder/coder/commit/bc609d0056adeb11b1d2dc282db4d0ad20f3444b --- tailnet/proto/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tailnet/proto/version.go b/tailnet/proto/version.go index 67ed79a0400bb..dd478fdcbdcd4 100644 --- a/tailnet/proto/version.go +++ b/tailnet/proto/version.go @@ -40,7 +40,7 @@ import ( // ScriptCompleted, but be prepared to process "unsupported" errors.) // // API v2.4: -// - Shipped in Coder v2.{{placeholder}} // TODO Vincent: Replace with the correct version +// - Shipped in Coder v2.20.0 // - Added support for GetResourcesMonitoringConfiguration and // PushResourcesMonitoringUsage RPCs on the Agent API. // - Added support for reporting connection events for auditing via the From b6182fe0547671b3c86760f73cd35d98cd6642eb Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 8 May 2025 15:09:16 +1000 Subject: [PATCH 12/46] chore: add code-insiders.svg static icon (#17716) We have `code.svg` but not `code-insiders.svg` --- site/src/theme/icons.json | 1 + site/static/icon/code-insiders.svg | 37 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 site/static/icon/code-insiders.svg diff --git a/site/src/theme/icons.json b/site/src/theme/icons.json index 9dcc10321682c..7a0d604167864 100644 --- a/site/src/theme/icons.json +++ b/site/src/theme/icons.json @@ -18,6 +18,7 @@ "centos.svg", "claude.svg", "clion.svg", + "code-insiders.svg", "code.svg", "coder.svg", "conda.svg", diff --git a/site/static/icon/code-insiders.svg b/site/static/icon/code-insiders.svg new file mode 100644 index 0000000000000..51175b9029ba3 --- /dev/null +++ b/site/static/icon/code-insiders.svg @@ -0,0 +1,37 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From c66e80e86276c48bbf17930f06a8679ac946e32e Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 8 May 2025 13:02:27 +0100 Subject: [PATCH 13/46] fix: extract checkbox label from dynamic parameter styling prop (#17651) resolves #17474 A label will only be shown next to the checkbox If there is a value for `label` in the styling prop for the dynamic parameter Screenshot 2025-05-01 at 21 35 32 --- go.mod | 2 +- go.sum | 4 ++-- site/src/api/typesGenerated.ts | 10 +++++++-- .../DynamicParameter/DynamicParameter.tsx | 22 +++++-------------- .../CreateWorkspacePageViewExperimental.tsx | 3 +-- 5 files changed, 17 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index cffcd99d06db8..536bce2fe28b7 100644 --- a/go.mod +++ b/go.mod @@ -488,7 +488,7 @@ require ( require ( github.com/anthropics/anthropic-sdk-go v0.2.0-beta.3 - github.com/coder/preview v0.0.1 + github.com/coder/preview v0.0.2-0.20250506154333-6f500ca7b245 github.com/fsnotify/fsnotify v1.9.0 github.com/kylecarbs/aisdk-go v0.0.8 github.com/mark3labs/mcp-go v0.25.0 diff --git a/go.sum b/go.sum index 4c418e5fd2a02..a3fc878ef2653 100644 --- a/go.sum +++ b/go.sum @@ -907,8 +907,8 @@ github.com/coder/pq v1.10.5-0.20240813183442-0c420cb5a048 h1:3jzYUlGH7ZELIH4XggX github.com/coder/pq v1.10.5-0.20240813183442-0c420cb5a048/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx9n47SZOKOpgSE1bbJzlE4qPVs= github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc= -github.com/coder/preview v0.0.1 h1:2X5McKdMOZJILTIDf7qRplXKupT+91qTJBN67XUh5cA= -github.com/coder/preview v0.0.1/go.mod h1:eInDmOdSDF8cxCvapIvYkGRzmzvcvGAFL1HYqcA4g+E= +github.com/coder/preview v0.0.2-0.20250506154333-6f500ca7b245 h1:RGoANNubwwPZF8puiYAk2qbzhVgipBMNu8WIrY1VIbI= +github.com/coder/preview v0.0.2-0.20250506154333-6f500ca7b245/go.mod h1:5VnO9yw7vq19hBgBqqBksE2BH53UTmNYH1QltkYLXJI= github.com/coder/quartz v0.1.2 h1:PVhc9sJimTdKd3VbygXtS4826EOCpB1fXoRlLnCrE+s= github.com/coder/quartz v0.1.2/go.mod h1:vsiCc+AHViMKH2CQpGIpFgdHIEQsxwm8yCscqKmzbRA= github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc= diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d195432f019c4..82332b76e060f 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1796,8 +1796,7 @@ export interface PreviewParameterData { readonly type: PreviewParameterType; // this is likely an enum in an external package "github.com/coder/terraform-provider-coder/v2/provider.ParameterFormType" readonly form_type: string; - // empty interface{} type, falling back to unknown - readonly styling: unknown; + readonly styling: PreviewParameterStyling; readonly mutable: boolean; readonly default_value: NullHCLString; readonly icon: string; @@ -1816,6 +1815,13 @@ export interface PreviewParameterOption { readonly icon: string; } +// From types/parameter.go +export interface PreviewParameterStyling { + readonly placeholder?: string; + readonly disabled?: boolean; + readonly label?: string; +} + // From types/enum.go export type PreviewParameterType = string; diff --git a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx index 9ec69158c4e84..5f8e875dbebcf 100644 --- a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx +++ b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx @@ -181,10 +181,7 @@ const ParameterField: FC = ({ > @@ -245,10 +242,7 @@ const ParameterField: FC = ({ onChange(JSON.stringify(values)); }} hidePlaceholderWhenSelected - placeholder={ - (parameter.styling as { placeholder?: string })?.placeholder || - "Select option" - } + placeholder={parameter.styling?.placeholder || "Select option"} emptyIndicator={

No results found @@ -304,9 +298,7 @@ const ParameterField: FC = ({ }} disabled={disabled} /> - +

); @@ -343,9 +335,7 @@ const ParameterField: FC = ({ target.style.height = `${target.scrollHeight}px`; }} disabled={disabled} - placeholder={ - (parameter.styling as { placeholder?: string })?.placeholder - } + placeholder={parameter.styling?.placeholder} required={parameter.required} /> ); @@ -377,9 +367,7 @@ const ParameterField: FC = ({ }} disabled={disabled} required={parameter.required} - placeholder={ - (parameter.styling as { placeholder?: string })?.placeholder - } + placeholder={parameter.styling?.placeholder} {...inputProps} /> ); diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index 58391cdad3d9f..0ba3ee9fb77f3 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -493,7 +493,6 @@ export const CreateWorkspacePageViewExperimental: FC<
{parameters.map((parameter, index) => { const parameterField = `rich_parameter_values.${index}`; - const parameterInputName = `${parameterField}.value`; const isPresetParameter = presetParameterNames.includes( parameter.name, ); @@ -501,7 +500,7 @@ export const CreateWorkspacePageViewExperimental: FC< disabledParams?.includes( parameter.name.toLowerCase().replace(/ /g, "_"), ) || - (parameter.styling as { disabled?: boolean })?.disabled || + parameter.styling?.disabled || creatingWorkspace || isPresetParameter; From 2695f4e9503319dfb5ea11f4e920d93de6c661fc Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 8 May 2025 09:32:32 -0300 Subject: [PATCH 14/46] chore: improve variable names of mocked users (#17701) Many times I got confused when using MockUser and MockUser2 so I just decided to better naming them to MockUserOwner and MockUserMember. --- .gitignore | 2 + docs/contributing/frontend.md | 2 +- .../OrganizationAutocomplete.stories.tsx | 6 +- .../UserAutocomplete.stories.tsx | 6 +- site/src/contexts/auth/RequireAuth.test.tsx | 6 +- site/src/hooks/useEmbeddedMetadata.test.ts | 6 +- .../dashboard/Navbar/MobileMenu.stories.tsx | 12 +-- .../dashboard/Navbar/NavbarView.stories.tsx | 10 +- .../dashboard/Navbar/NavbarView.test.tsx | 10 +- .../dashboard/Navbar/ProxyMenu.stories.tsx | 4 +- .../UserDropdown/UserDropdown.stories.tsx | 4 +- .../UserDropdown/UserDropdownContent.test.tsx | 6 +- .../AuditLogRow/AuditLogRow.stories.tsx | 4 +- .../pages/AuditPage/AuditPageView.stories.tsx | 4 +- .../CreateWorkspacePage.test.tsx | 14 +-- .../CreateWorkspacePageView.stories.tsx | 4 +- ...eWorkspacePageViewExperimental.stories.tsx | 4 +- .../NotificationsPage/storybookUtils.ts | 4 +- .../OrganizationMembersPage.test.tsx | 10 +- .../OrganizationMembersPageView.stories.tsx | 4 +- site/src/pages/SetupPage/SetupPage.test.tsx | 4 +- .../TerminalPage/TerminalPage.stories.tsx | 4 +- .../pages/TerminalPage/TerminalPage.test.tsx | 8 +- .../AccountPage/AccountForm.test.tsx | 14 +-- .../AccountPage/AccountUserGroups.stories.tsx | 4 +- .../AppearancePage/AppearancePage.test.tsx | 12 +-- .../NotificationsPage.stories.tsx | 6 +- .../SchedulePage/SchedulePage.test.tsx | 10 +- .../UsersPage/ResetPasswordDialog.stories.tsx | 4 +- .../src/pages/UsersPage/UsersPage.stories.tsx | 4 +- .../pages/UsersPage/UsersPageView.stories.tsx | 6 +- .../UsersTable/UsersTable.stories.tsx | 20 ++-- .../pages/WorkspacePage/Workspace.stories.tsx | 2 +- .../WorkspaceActions.stories.tsx | 6 +- .../WorkspacePage/WorkspacePage.test.tsx | 4 +- .../WorkspacePage/WorkspaceTopbar.stories.tsx | 8 +- .../WorkspaceSchedulePage.test.tsx | 8 +- .../BatchDeleteConfirmation.stories.tsx | 10 +- .../BatchUpdateConfirmation.stories.tsx | 6 +- .../WorkspacesPageView.stories.tsx | 6 +- site/src/testHelpers/entities.ts | 95 +++++++++---------- site/src/testHelpers/handlers.ts | 8 +- site/src/testHelpers/renderHelpers.tsx | 10 +- 43 files changed, 189 insertions(+), 192 deletions(-) diff --git a/.gitignore b/.gitignore index 8d29eff1048d1..24021e54ddde2 100644 --- a/.gitignore +++ b/.gitignore @@ -82,3 +82,5 @@ result # dlv debug binaries for go tests __debug_bin* + +**/.claude/settings.local.json diff --git a/docs/contributing/frontend.md b/docs/contributing/frontend.md index 711246b0277d8..62e86c9ad4ab9 100644 --- a/docs/contributing/frontend.md +++ b/docs/contributing/frontend.md @@ -131,7 +131,7 @@ export const WithQuota: Story = { parameters: { queries: [ { - key: getWorkspaceQuotaQueryKey(MockUser.username), + key: getWorkspaceQuotaQueryKey(MockUserOwner.username), data: { credits_consumed: 2, budget: 40, diff --git a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx index 87a7c544366a8..949b293dfce04 100644 --- a/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx +++ b/site/src/components/OrganizationAutocomplete/OrganizationAutocomplete.stories.tsx @@ -4,7 +4,7 @@ import { userEvent, within } from "@storybook/test"; import { MockOrganization, MockOrganization2, - MockUser, + MockUserOwner, } from "testHelpers/entities"; import { OrganizationAutocomplete } from "./OrganizationAutocomplete"; @@ -22,7 +22,7 @@ type Story = StoryObj; export const ManyOrgs: Story = { parameters: { showOrganizations: true, - user: MockUser, + user: MockUserOwner, features: ["multiple_organizations"], permissions: { viewDeploymentConfig: true }, queries: [ @@ -42,7 +42,7 @@ export const ManyOrgs: Story = { export const OneOrg: Story = { parameters: { showOrganizations: true, - user: MockUser, + user: MockUserOwner, features: ["multiple_organizations"], permissions: { viewDeploymentConfig: true }, queries: [ diff --git a/site/src/components/UserAutocomplete/UserAutocomplete.stories.tsx b/site/src/components/UserAutocomplete/UserAutocomplete.stories.tsx index eee96b248f52b..06c16e22fdebe 100644 --- a/site/src/components/UserAutocomplete/UserAutocomplete.stories.tsx +++ b/site/src/components/UserAutocomplete/UserAutocomplete.stories.tsx @@ -1,5 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react"; -import { MockUser } from "testHelpers/entities"; +import { MockUserOwner } from "testHelpers/entities"; import { UserAutocomplete } from "./UserAutocomplete"; const meta: Meta = { @@ -12,13 +12,13 @@ type Story = StoryObj; export const WithLabel: Story = { args: { - value: MockUser, + value: MockUserOwner, label: "User", }, }; export const NoLabel: Story = { args: { - value: MockUser, + value: MockUserOwner, }, }; diff --git a/site/src/contexts/auth/RequireAuth.test.tsx b/site/src/contexts/auth/RequireAuth.test.tsx index 291d442adbc04..b24bb06cb055c 100644 --- a/site/src/contexts/auth/RequireAuth.test.tsx +++ b/site/src/contexts/auth/RequireAuth.test.tsx @@ -3,7 +3,7 @@ import { useAuthenticated } from "hooks"; import { http, HttpResponse } from "msw"; import type { FC, PropsWithChildren } from "react"; import { QueryClientProvider } from "react-query"; -import { MockPermissions, MockUser } from "testHelpers/entities"; +import { MockPermissions, MockUserOwner } from "testHelpers/entities"; import { createTestQueryClient, renderWithAuth, @@ -82,7 +82,7 @@ describe("useAuthenticated", () => { expect(() => { renderHook(() => useAuthenticated(), { - wrapper: createAuthWrapper({ user: MockUser }), + wrapper: createAuthWrapper({ user: MockUserOwner }), }); }).toThrow("Permissions are not available."); @@ -93,7 +93,7 @@ describe("useAuthenticated", () => { expect(() => { renderHook(() => useAuthenticated(), { wrapper: createAuthWrapper({ - user: MockUser, + user: MockUserOwner, permissions: MockPermissions, }), }); diff --git a/site/src/hooks/useEmbeddedMetadata.test.ts b/site/src/hooks/useEmbeddedMetadata.test.ts index aacb635ada3bf..6f7b2741ed96b 100644 --- a/site/src/hooks/useEmbeddedMetadata.test.ts +++ b/site/src/hooks/useEmbeddedMetadata.test.ts @@ -5,8 +5,8 @@ import { MockBuildInfo, MockEntitlements, MockExperiments, - MockUser, MockUserAppearanceSettings, + MockUserOwner, } from "testHelpers/entities"; import { DEFAULT_METADATA_KEY, @@ -38,7 +38,7 @@ const mockDataForTags = { "build-info": MockBuildInfo, entitlements: MockEntitlements, experiments: MockExperiments, - user: MockUser, + user: MockUserOwner, userAppearance: MockUserAppearanceSettings, regions: MockRegions, } as const satisfies Record; @@ -97,7 +97,7 @@ const populatedMetadata: RuntimeHtmlMetadata = { }, user: { available: true, - value: MockUser, + value: MockUserOwner, }, userAppearance: { available: true, diff --git a/site/src/modules/dashboard/Navbar/MobileMenu.stories.tsx b/site/src/modules/dashboard/Navbar/MobileMenu.stories.tsx index 5392ecaaee6c9..058c8799c95e0 100644 --- a/site/src/modules/dashboard/Navbar/MobileMenu.stories.tsx +++ b/site/src/modules/dashboard/Navbar/MobileMenu.stories.tsx @@ -6,8 +6,8 @@ import { MockPrimaryWorkspaceProxy, MockProxyLatencies, MockSupportLinks, - MockUser, - MockUser2, + MockUserMember, + MockUserOwner, MockWorkspaceProxies, } from "testHelpers/entities"; import { MobileMenu } from "./MobileMenu"; @@ -36,7 +36,7 @@ const meta: Meta = { proxyLatencies: MockProxyLatencies, proxies: MockWorkspaceProxies, }, - user: MockUser, + user: MockUserOwner, supportLinks: MockSupportLinks, onSignOut: fn(), isDefaultOpen: true, @@ -63,7 +63,7 @@ export const Admin: Story = { export const Auditor: Story = { args: { - user: MockUser2, + user: MockUserMember, canViewAuditLog: true, canViewDeployment: false, canViewHealth: false, @@ -74,7 +74,7 @@ export const Auditor: Story = { export const OrgAdmin: Story = { args: { - user: MockUser2, + user: MockUserMember, canViewAuditLog: true, canViewDeployment: false, canViewHealth: false, @@ -85,7 +85,7 @@ export const OrgAdmin: Story = { export const Member: Story = { args: { - user: MockUser2, + user: MockUserMember, canViewAuditLog: false, canViewDeployment: false, canViewHealth: false, diff --git a/site/src/modules/dashboard/Navbar/NavbarView.stories.tsx b/site/src/modules/dashboard/Navbar/NavbarView.stories.tsx index ae13c7fcc9129..6bd076a1c1c68 100644 --- a/site/src/modules/dashboard/Navbar/NavbarView.stories.tsx +++ b/site/src/modules/dashboard/Navbar/NavbarView.stories.tsx @@ -1,7 +1,7 @@ import type { Meta, StoryObj } from "@storybook/react"; import { userEvent, within } from "@storybook/test"; import { chromaticWithTablet } from "testHelpers/chromatic"; -import { MockUser, MockUser2 } from "testHelpers/entities"; +import { MockUserMember, MockUserOwner } from "testHelpers/entities"; import { withDashboardProvider } from "testHelpers/storybook"; import { NavbarView } from "./NavbarView"; @@ -10,7 +10,7 @@ const meta: Meta = { parameters: { chromatic: chromaticWithTablet, layout: "fullscreen" }, component: NavbarView, args: { - user: MockUser, + user: MockUserOwner, canViewAuditLog: true, canViewDeployment: true, canViewHealth: true, @@ -33,7 +33,7 @@ export const ForAdmin: Story = { export const ForAuditor: Story = { args: { - user: MockUser2, + user: MockUserMember, canViewAuditLog: true, canViewDeployment: false, canViewHealth: false, @@ -49,7 +49,7 @@ export const ForAuditor: Story = { export const ForOrgAdmin: Story = { args: { - user: MockUser2, + user: MockUserMember, canViewAuditLog: true, canViewDeployment: false, canViewHealth: false, @@ -65,7 +65,7 @@ export const ForOrgAdmin: Story = { export const ForMember: Story = { args: { - user: MockUser2, + user: MockUserMember, canViewAuditLog: false, canViewDeployment: false, canViewHealth: false, diff --git a/site/src/modules/dashboard/Navbar/NavbarView.test.tsx b/site/src/modules/dashboard/Navbar/NavbarView.test.tsx index 4cb15ae78621b..6739f666c2b17 100644 --- a/site/src/modules/dashboard/Navbar/NavbarView.test.tsx +++ b/site/src/modules/dashboard/Navbar/NavbarView.test.tsx @@ -1,7 +1,7 @@ import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import type { ProxyContextValue } from "contexts/ProxyContext"; -import { MockPrimaryWorkspaceProxy, MockUser } from "testHelpers/entities"; +import { MockPrimaryWorkspaceProxy, MockUserOwner } from "testHelpers/entities"; import { renderWithAuth } from "testHelpers/renderHelpers"; import { NavbarView } from "./NavbarView"; @@ -26,7 +26,7 @@ describe("NavbarView", () => { renderWithAuth( { renderWithAuth( { renderWithAuth( { renderWithAuth( = { ], parameters: { queries: [ - { key: ["me"], data: MockUser }, + { key: ["me"], data: MockUserOwner }, { key: ["authMethods"], data: MockAuthMethodsAll }, { key: ["hasFirstUser"], data: true }, { diff --git a/site/src/modules/dashboard/Navbar/UserDropdown/UserDropdown.stories.tsx b/site/src/modules/dashboard/Navbar/UserDropdown/UserDropdown.stories.tsx index ff35d79807287..24ffe6adf8ca6 100644 --- a/site/src/modules/dashboard/Navbar/UserDropdown/UserDropdown.stories.tsx +++ b/site/src/modules/dashboard/Navbar/UserDropdown/UserDropdown.stories.tsx @@ -1,6 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react"; import { expect, screen, userEvent, waitFor, within } from "@storybook/test"; -import { MockBuildInfo, MockUser } from "testHelpers/entities"; +import { MockBuildInfo, MockUserOwner } from "testHelpers/entities"; import { withDashboardProvider } from "testHelpers/storybook"; import { UserDropdown } from "./UserDropdown"; @@ -8,7 +8,7 @@ const meta: Meta = { title: "modules/dashboard/UserDropdown", component: UserDropdown, args: { - user: MockUser, + user: MockUserOwner, buildInfo: MockBuildInfo, supportLinks: [ { icon: "docs", name: "Documentation", target: "" }, diff --git a/site/src/modules/dashboard/Navbar/UserDropdown/UserDropdownContent.test.tsx b/site/src/modules/dashboard/Navbar/UserDropdown/UserDropdownContent.test.tsx index d4f3858d17fef..6a9018c4eeeca 100644 --- a/site/src/modules/dashboard/Navbar/UserDropdown/UserDropdownContent.test.tsx +++ b/site/src/modules/dashboard/Navbar/UserDropdown/UserDropdownContent.test.tsx @@ -1,6 +1,6 @@ import { screen } from "@testing-library/react"; import { Popover } from "components/deprecated/Popover/Popover"; -import { MockUser } from "testHelpers/entities"; +import { MockUserOwner } from "testHelpers/entities"; import { render, waitForLoaderToBeRemoved } from "testHelpers/renderHelpers"; import { Language, UserDropdownContent } from "./UserDropdownContent"; @@ -8,7 +8,7 @@ describe("UserDropdownContent", () => { it("has the correct link for the account item", async () => { render( - + , ); await waitForLoaderToBeRemoved(); @@ -25,7 +25,7 @@ describe("UserDropdownContent", () => { const onSignOut = jest.fn(); render( - + , ); await waitForLoaderToBeRemoved(); diff --git a/site/src/pages/AuditPage/AuditLogRow/AuditLogRow.stories.tsx b/site/src/pages/AuditPage/AuditLogRow/AuditLogRow.stories.tsx index 8bb45aa39378b..ab5e55f8bbd84 100644 --- a/site/src/pages/AuditPage/AuditLogRow/AuditLogRow.stories.tsx +++ b/site/src/pages/AuditPage/AuditLogRow/AuditLogRow.stories.tsx @@ -13,7 +13,7 @@ import { MockAuditLogRequestPasswordReset, MockAuditLogWithDeletedResource, MockAuditLogWithWorkspaceBuild, - MockUser, + MockUserOwner, } from "testHelpers/entities"; import { AuditLogRow } from "./AuditLogRow"; @@ -155,7 +155,7 @@ export const NoUserAgent: Story = { description: "{user} deleted workspace {target}", resource_link: "/@jon/yeee/builds/35", is_deleted: false, - user: MockUser, + user: MockUserOwner, }, }, }; diff --git a/site/src/pages/AuditPage/AuditPageView.stories.tsx b/site/src/pages/AuditPage/AuditPageView.stories.tsx index c7830ccca4533..323ae6d78bde8 100644 --- a/site/src/pages/AuditPage/AuditPageView.stories.tsx +++ b/site/src/pages/AuditPage/AuditPageView.stories.tsx @@ -14,7 +14,7 @@ import { MockAuditLog, MockAuditLog2, MockAuditLog3, - MockUser, + MockUserOwner, } from "testHelpers/entities"; import { AuditPageView } from "./AuditPageView"; @@ -23,7 +23,7 @@ type FilterProps = ComponentProps["filterProps"]; const defaultFilterProps = getDefaultFilterProps({ query: "owner:me", values: { - username: MockUser.username, + username: MockUserOwner.username, action: undefined, resource_type: undefined, organization: undefined, diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index 64deba2116fb1..868aa85c751bd 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -8,7 +8,7 @@ import { MockTemplateVersionParameter1, MockTemplateVersionParameter2, MockTemplateVersionParameter3, - MockUser, + MockUserOwner, MockWorkspace, MockWorkspaceQuota, MockWorkspaceRequest, @@ -36,7 +36,7 @@ describe("CreateWorkspacePage", () => { it("succeeds with default owner", async () => { jest .spyOn(API, "getUsers") - .mockResolvedValueOnce({ users: [MockUser], count: 1 }); + .mockResolvedValueOnce({ users: [MockUserOwner], count: 1 }); jest .spyOn(API, "getWorkspaceQuota") .mockResolvedValueOnce(MockWorkspaceQuota); @@ -59,7 +59,7 @@ describe("CreateWorkspacePage", () => { await waitFor(() => expect(API.createWorkspace).toBeCalledWith( - MockUser.id, + MockUserOwner.id, expect.objectContaining({ ...MockWorkspaceRichParametersRequest, }), @@ -186,7 +186,7 @@ describe("CreateWorkspacePage", () => { .mockResolvedValueOnce(MockWorkspaceQuota); jest .spyOn(API, "getUsers") - .mockResolvedValueOnce({ users: [MockUser], count: 1 }); + .mockResolvedValueOnce({ users: [MockUserOwner], count: 1 }); jest.spyOn(API, "createWorkspace").mockResolvedValueOnce(MockWorkspace); jest .spyOn(API, "getTemplateVersionExternalAuth") @@ -219,7 +219,7 @@ describe("CreateWorkspacePage", () => { await waitFor(() => expect(API.createWorkspace).toBeCalledWith( - MockUser.id, + MockUserOwner.id, expect.objectContaining({ ...MockWorkspaceRequest, }), @@ -233,7 +233,7 @@ describe("CreateWorkspacePage", () => { .mockResolvedValueOnce(MockWorkspaceQuota); jest .spyOn(API, "getUsers") - .mockResolvedValueOnce({ users: [MockUser], count: 1 }); + .mockResolvedValueOnce({ users: [MockUserOwner], count: 1 }); jest.spyOn(API, "createWorkspace").mockResolvedValueOnce(MockWorkspace); jest .spyOn(API, "getTemplateVersionExternalAuth") @@ -258,7 +258,7 @@ describe("CreateWorkspacePage", () => { await waitFor(() => expect(API.createWorkspace).toBeCalledWith( - MockUser.id, + MockUserOwner.id, expect.objectContaining({ ...MockWorkspaceRequest, }), diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index 564209f076b08..db0a548ccc313 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -8,7 +8,7 @@ import { MockTemplateVersionParameter1, MockTemplateVersionParameter2, MockTemplateVersionParameter3, - MockUser, + MockUserOwner, mockApiError, } from "testHelpers/entities"; import { CreateWorkspacePageView } from "./CreateWorkspacePageView"; @@ -19,7 +19,7 @@ const meta: Meta = { component: CreateWorkspacePageView, args: { defaultName: "", - defaultOwner: MockUser, + defaultOwner: MockUserOwner, autofillParameters: [], template: MockTemplate, parameters: [], diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.stories.tsx index a41e3a48c0ad9..e00b04fd6bf50 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.stories.tsx @@ -1,7 +1,7 @@ import type { Meta, StoryObj } from "@storybook/react"; import { DetailedError } from "api/errors"; import { chromatic } from "testHelpers/chromatic"; -import { MockTemplate, MockUser } from "testHelpers/entities"; +import { MockTemplate, MockUserOwner } from "testHelpers/entities"; import { CreateWorkspacePageViewExperimental } from "./CreateWorkspacePageViewExperimental"; const meta: Meta = { @@ -12,7 +12,7 @@ const meta: Meta = { autofillParameters: [], diagnostics: [], defaultName: "", - defaultOwner: MockUser, + defaultOwner: MockUserOwner, externalAuth: [], externalAuthPollingState: "idle", hasAllRequiredExternalAuth: true, diff --git a/site/src/pages/DeploymentSettingsPage/NotificationsPage/storybookUtils.ts b/site/src/pages/DeploymentSettingsPage/NotificationsPage/storybookUtils.ts index 7f344d457817f..f27535d5b5397 100644 --- a/site/src/pages/DeploymentSettingsPage/NotificationsPage/storybookUtils.ts +++ b/site/src/pages/DeploymentSettingsPage/NotificationsPage/storybookUtils.ts @@ -7,7 +7,7 @@ import type { DeploymentValues, SerpentOption } from "api/typesGenerated"; import { MockNotificationMethodsResponse, MockNotificationTemplates, - MockUser, + MockUserOwner, } from "testHelpers/entities"; import { withAuthProvider, @@ -193,7 +193,7 @@ export const baseMeta = { data: MockNotificationMethodsResponse, }, ], - user: MockUser, + user: MockUserOwner, permissions: { viewDeploymentConfig: true }, deploymentOptions: mockNotificationsDeploymentOptions, deploymentValues: { diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationMembersPage.test.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationMembersPage.test.tsx index 660e66ca0ccb2..4c90a21659ee2 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationMembersPage.test.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationMembersPage.test.tsx @@ -7,7 +7,7 @@ import { MockOrganization, MockOrganizationAuditorRole, MockOrganizationPermissions, - MockUser, + MockUserOwner, } from "testHelpers/entities"; import { renderWithOrganizationSettingsLayout, @@ -102,11 +102,11 @@ describe("OrganizationMembersPage", () => { it("updates the roles", async () => { server.use( http.put( - `/api/v2/organizations/:organizationId/members/${MockUser.id}/roles`, + `/api/v2/organizations/:organizationId/members/${MockUserOwner.id}/roles`, async () => { return HttpResponse.json({ - ...MockUser, - roles: [...MockUser.roles, MockOrganizationAuditorRole], + ...MockUserOwner, + roles: [...MockUserOwner.roles, MockOrganizationAuditorRole], }); }, ), @@ -122,7 +122,7 @@ describe("OrganizationMembersPage", () => { it("shows an error message", async () => { server.use( http.put( - `/api/v2/organizations/:organizationId/members/${MockUser.id}/roles`, + `/api/v2/organizations/:organizationId/members/${MockUserOwner.id}/roles`, () => { return HttpResponse.json( { message: "Error on updating the user roles." }, diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationMembersPageView.stories.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationMembersPageView.stories.tsx index 1c2f2c6e804a3..566bebfe7f3af 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationMembersPageView.stories.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationMembersPageView.stories.tsx @@ -4,7 +4,7 @@ import type { UsePaginatedQueryResult } from "hooks/usePaginatedQuery"; import { MockOrganizationMember, MockOrganizationMember2, - MockUser, + MockUserOwner, } from "testHelpers/entities"; import { OrganizationMembersPageView } from "./OrganizationMembersPageView"; @@ -17,7 +17,7 @@ const meta: Meta = { isAddingMember: false, isUpdatingMemberRoles: false, canViewMembers: true, - me: MockUser, + me: MockUserOwner, members: [ { ...MockOrganizationMember, groups: [] }, { ...MockOrganizationMember2, groups: [] }, diff --git a/site/src/pages/SetupPage/SetupPage.test.tsx b/site/src/pages/SetupPage/SetupPage.test.tsx index 47cf1d58746e2..0ab5d15c6f338 100644 --- a/site/src/pages/SetupPage/SetupPage.test.tsx +++ b/site/src/pages/SetupPage/SetupPage.test.tsx @@ -3,7 +3,7 @@ import userEvent from "@testing-library/user-event"; import type { Response, User } from "api/typesGenerated"; import { http, HttpResponse } from "msw"; import { createMemoryRouter } from "react-router-dom"; -import { MockBuildInfo, MockUser } from "testHelpers/entities"; +import { MockBuildInfo, MockUserOwner } from "testHelpers/entities"; import { renderWithRouter, waitForLoaderToBeRemoved, @@ -83,7 +83,7 @@ describe("Setup Page", () => { { status: 401 }, ); } - return HttpResponse.json(MockUser); + return HttpResponse.json(MockUserOwner); }), http.get( "/api/v2/users/first", diff --git a/site/src/pages/TerminalPage/TerminalPage.stories.tsx b/site/src/pages/TerminalPage/TerminalPage.stories.tsx index d58f3e328e3ff..298f890637042 100644 --- a/site/src/pages/TerminalPage/TerminalPage.stories.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.stories.tsx @@ -17,8 +17,8 @@ import { MockDeploymentConfig, MockEntitlements, MockExperiments, - MockUser, MockUserAppearanceSettings, + MockUserOwner, MockWorkspace, MockWorkspaceAgent, } from "testHelpers/entities"; @@ -66,7 +66,7 @@ const meta = { ), }), queries: [ - { key: ["me"], data: MockUser }, + { key: ["me"], data: MockUserOwner }, { key: ["authMethods"], data: MockAuthMethodsAll }, { key: ["hasFirstUser"], data: true }, { key: ["buildInfo"], data: MockBuildInfo }, diff --git a/site/src/pages/TerminalPage/TerminalPage.test.tsx b/site/src/pages/TerminalPage/TerminalPage.test.tsx index 7c1e6a154fa0d..7600fa5257d43 100644 --- a/site/src/pages/TerminalPage/TerminalPage.test.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.test.tsx @@ -4,7 +4,7 @@ import { API } from "api/api"; import WS from "jest-websocket-mock"; import { http, HttpResponse } from "msw"; import { - MockUser, + MockUserOwner, MockWorkspace, MockWorkspaceAgent, } from "testHelpers/entities"; @@ -13,7 +13,7 @@ import { server } from "testHelpers/server"; import TerminalPage, { Language } from "./TerminalPage"; const renderTerminal = async ( - route = `/${MockUser.username}/${MockWorkspace.name}/terminal`, + route = `/${MockUserOwner.username}/${MockWorkspace.name}/terminal`, ) => { const utils = renderWithAuth(, { route, @@ -62,11 +62,11 @@ describe("TerminalPage", () => { `ws://localhost/api/v2/workspaceagents/${MockWorkspaceAgent.id}/pty`, ); await renderTerminal( - `/${MockUser.username}/${MockWorkspace.name}/terminal`, + `/${MockUserOwner.username}/${MockWorkspace.name}/terminal`, ); await waitFor(() => { expect(API.getWorkspaceByOwnerAndName).toHaveBeenCalledWith( - MockUser.username, + MockUserOwner.username, MockWorkspace.name, { include_deleted: true }, ); diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountForm.test.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountForm.test.tsx index fca96c4e82200..6c50ba8addc5c 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountForm.test.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountForm.test.tsx @@ -1,6 +1,6 @@ import { screen } from "@testing-library/react"; import type { UpdateUserProfileRequest } from "api/typesGenerated"; -import { MockUser2 } from "testHelpers/entities"; +import { MockUserMember } from "testHelpers/entities"; import { render } from "testHelpers/renderHelpers"; import { AccountForm } from "./AccountForm"; @@ -12,15 +12,15 @@ describe("AccountForm", () => { it("allows updating username", async () => { // Given const mockInitialValues: UpdateUserProfileRequest = { - username: MockUser2.username, - name: MockUser2.name, + username: MockUserMember.username, + name: MockUserMember.name, }; // When render( { @@ -43,15 +43,15 @@ describe("AccountForm", () => { it("does not allow updating username", async () => { // Given const mockInitialValues: UpdateUserProfileRequest = { - username: MockUser2.username, - name: MockUser2.name, + username: MockUserMember.username, + name: MockUserMember.name, }; // When render( { diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountUserGroups.stories.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountUserGroups.stories.tsx index 6421f73e5c79c..a85327e420e3a 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountUserGroups.stories.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountUserGroups.stories.tsx @@ -1,7 +1,7 @@ import type { Meta, StoryObj } from "@storybook/react"; import { MockGroup as MockGroup1, - MockUser, + MockUserOwner, mockApiError, } from "testHelpers/entities"; import { withDashboardProvider } from "testHelpers/storybook"; @@ -11,7 +11,7 @@ const MockGroup2 = { ...MockGroup1, avatar_url: "", display_name: "Goofy Goobers", - members: [MockUser], + members: [MockUserOwner], }; const mockError = mockApiError({ diff --git a/site/src/pages/UserSettingsPage/AppearancePage/AppearancePage.test.tsx b/site/src/pages/UserSettingsPage/AppearancePage/AppearancePage.test.tsx index ade7c55f51417..e6c2462acfabc 100644 --- a/site/src/pages/UserSettingsPage/AppearancePage/AppearancePage.test.tsx +++ b/site/src/pages/UserSettingsPage/AppearancePage/AppearancePage.test.tsx @@ -1,7 +1,7 @@ import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { API } from "api/api"; -import { MockUser } from "testHelpers/entities"; +import { MockUserOwner } from "testHelpers/entities"; import { renderWithAuth } from "testHelpers/renderHelpers"; import AppearancePage from "./AppearancePage"; @@ -10,7 +10,7 @@ describe("appearance page", () => { renderWithAuth(); jest.spyOn(API, "updateAppearanceSettings").mockResolvedValueOnce({ - ...MockUser, + ...MockUserOwner, theme_preference: "dark", terminal_font: "fira-code", }); @@ -26,7 +26,7 @@ describe("appearance page", () => { renderWithAuth(); jest.spyOn(API, "updateAppearanceSettings").mockResolvedValueOnce({ - ...MockUser, + ...MockUserOwner, terminal_font: "ibm-plex-mono", theme_preference: "light", }); @@ -46,7 +46,7 @@ describe("appearance page", () => { renderWithAuth(); jest.spyOn(API, "updateAppearanceSettings").mockResolvedValueOnce({ - ...MockUser, + ...MockUserOwner, terminal_font: "fira-code", theme_preference: "dark", }); @@ -69,12 +69,12 @@ describe("appearance page", () => { jest .spyOn(API, "updateAppearanceSettings") .mockResolvedValueOnce({ - ...MockUser, + ...MockUserOwner, terminal_font: "fira-code", theme_preference: "dark", }) .mockResolvedValueOnce({ - ...MockUser, + ...MockUserOwner, terminal_font: "ibm-plex-mono", theme_preference: "dark", }); diff --git a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.stories.tsx b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.stories.tsx index 14492eeefe68c..e2ac02e773d2d 100644 --- a/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.stories.tsx +++ b/site/src/pages/UserSettingsPage/NotificationsPage/NotificationsPage.stories.tsx @@ -11,7 +11,7 @@ import { MockNotificationMethodsResponse, MockNotificationPreferences, MockNotificationTemplates, - MockUser, + MockUserOwner, } from "testHelpers/entities"; import { withAuthProvider, @@ -27,7 +27,7 @@ const meta = { experiments: ["notifications"], queries: [ { - key: userNotificationPreferencesKey(MockUser.id), + key: userNotificationPreferencesKey(MockUserOwner.id), data: MockNotificationPreferences, }, { @@ -39,7 +39,7 @@ const meta = { data: MockNotificationMethodsResponse, }, ], - user: MockUser, + user: MockUserOwner, permissions: { viewDeploymentConfig: true }, }, decorators: [withGlobalSnackbar, withAuthProvider, withDashboardProvider], diff --git a/site/src/pages/UserSettingsPage/SchedulePage/SchedulePage.test.tsx b/site/src/pages/UserSettingsPage/SchedulePage/SchedulePage.test.tsx index b089c36543490..874dbf3c7fb32 100644 --- a/site/src/pages/UserSettingsPage/SchedulePage/SchedulePage.test.tsx +++ b/site/src/pages/UserSettingsPage/SchedulePage/SchedulePage.test.tsx @@ -2,7 +2,7 @@ import { fireEvent, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import type { UpdateUserQuietHoursScheduleRequest } from "api/typesGenerated"; import { http, HttpResponse } from "msw"; -import { MockUser } from "testHelpers/entities"; +import { MockUserOwner } from "testHelpers/entities"; import { renderWithAuth } from "testHelpers/renderHelpers"; import { server } from "testHelpers/server"; import SchedulePage from "./SchedulePage"; @@ -55,7 +55,7 @@ const cronTests = [ describe("SchedulePage", () => { beforeEach(() => { server.use( - http.get(`/api/v2/users/${MockUser.id}/quiet-hours`, () => { + http.get(`/api/v2/users/${MockUserOwner.id}/quiet-hours`, () => { return HttpResponse.json(defaultQuietHoursResponse); }), ); @@ -67,7 +67,7 @@ describe("SchedulePage", () => { async (test) => { server.use( http.put( - `/api/v2/users/${MockUser.id}/quiet-hours`, + `/api/v2/users/${MockUserOwner.id}/quiet-hours`, async ({ request }) => { const data = (await request.json()) as UpdateUserQuietHoursScheduleRequest; @@ -98,7 +98,7 @@ describe("SchedulePage", () => { describe("when it is an unknown error", () => { it("shows a generic error message", async () => { server.use( - http.put(`/api/v2/users/${MockUser.id}/quiet-hours`, () => { + http.put(`/api/v2/users/${MockUserOwner.id}/quiet-hours`, () => { return HttpResponse.json( { message: "oh no!", @@ -120,7 +120,7 @@ describe("SchedulePage", () => { describe("when user custom schedule is disabled", () => { it("shows a warning and disables the form", async () => { server.use( - http.get(`/api/v2/users/${MockUser.id}/quiet-hours`, () => { + http.get(`/api/v2/users/${MockUserOwner.id}/quiet-hours`, () => { return HttpResponse.json({ raw_schedule: "CRON_TZ=America/Chicago 0 0 * * *", user_can_set: false, diff --git a/site/src/pages/UsersPage/ResetPasswordDialog.stories.tsx b/site/src/pages/UsersPage/ResetPasswordDialog.stories.tsx index 633c5ab4ca4d3..bd64eef6ae7e5 100644 --- a/site/src/pages/UsersPage/ResetPasswordDialog.stories.tsx +++ b/site/src/pages/UsersPage/ResetPasswordDialog.stories.tsx @@ -1,5 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react"; -import { MockUser } from "testHelpers/entities"; +import { MockUserOwner } from "testHelpers/entities"; import { ResetPasswordDialog } from "./ResetPasswordDialog"; const meta: Meta = { @@ -13,7 +13,7 @@ type Story = StoryObj; const Example: Story = { args: { open: true, - user: MockUser, + user: MockUserOwner, newPassword: "somerandomstringhere", onConfirm: () => {}, onClose: () => {}, diff --git a/site/src/pages/UsersPage/UsersPage.stories.tsx b/site/src/pages/UsersPage/UsersPage.stories.tsx index 88059c35e3096..fdede4ec9f163 100644 --- a/site/src/pages/UsersPage/UsersPage.stories.tsx +++ b/site/src/pages/UsersPage/UsersPage.stories.tsx @@ -9,7 +9,7 @@ import type { User } from "api/typesGenerated"; import { MockGroups } from "pages/UsersPage/storybookData/groups"; import { MockRoles } from "pages/UsersPage/storybookData/roles"; import { MockUsers } from "pages/UsersPage/storybookData/users"; -import { MockAuthMethodsAll, MockUser } from "testHelpers/entities"; +import { MockAuthMethodsAll, MockUserOwner } from "testHelpers/entities"; import { withAuthProvider, withDashboardProvider, @@ -59,7 +59,7 @@ const parameters = { }, }, ], - user: MockUser, + user: MockUserOwner, permissions: { createUser: true, updateUsers: true, diff --git a/site/src/pages/UsersPage/UsersPageView.stories.tsx b/site/src/pages/UsersPage/UsersPageView.stories.tsx index 81e557221edff..c15b8aefc1b23 100644 --- a/site/src/pages/UsersPage/UsersPageView.stories.tsx +++ b/site/src/pages/UsersPage/UsersPageView.stories.tsx @@ -9,8 +9,8 @@ import type { ComponentProps } from "react"; import { MockAssignableSiteRoles, MockAuthMethodsPasswordOnly, - MockUser, - MockUser2, + MockUserMember, + MockUserOwner, mockApiError, } from "testHelpers/entities"; import { UsersPageView } from "./UsersPageView"; @@ -32,7 +32,7 @@ const meta: Meta = { component: UsersPageView, args: { isNonInitialPage: false, - users: [MockUser, MockUser2], + users: [MockUserOwner, MockUserMember], roles: MockAssignableSiteRoles, canEditUsers: true, filterProps: defaultFilterProps, diff --git a/site/src/pages/UsersPage/UsersTable/UsersTable.stories.tsx b/site/src/pages/UsersPage/UsersTable/UsersTable.stories.tsx index 4375e69cc77ca..5ef7116025919 100644 --- a/site/src/pages/UsersPage/UsersTable/UsersTable.stories.tsx +++ b/site/src/pages/UsersPage/UsersTable/UsersTable.stories.tsx @@ -6,15 +6,15 @@ import { MockGroup, MockMemberRole, MockTemplateAdminRole, - MockUser, - MockUser2, MockUserAdminRole, + MockUserMember, + MockUserOwner, } from "testHelpers/entities"; import { UsersTable } from "./UsersTable"; const mockGroupsByUserId = new Map([ - [MockUser.id, [MockGroup]], - [MockUser2.id, [MockGroup]], + [MockUserOwner.id, [MockGroup]], + [MockUserMember.id, [MockGroup]], ]); const meta: Meta = { @@ -31,7 +31,7 @@ type Story = StoryObj; export const Example: Story = { args: { - users: [MockUser, MockUser2], + users: [MockUserOwner, MockUserMember], roles: MockAssignableSiteRoles, canEditUsers: false, groupsByUserId: mockGroupsByUserId, @@ -41,10 +41,10 @@ export const Example: Story = { export const Editable: Story = { args: { users: [ - MockUser, - MockUser2, + MockUserOwner, + MockUserMember, { - ...MockUser, + ...MockUserOwner, username: "John Doe", email: "john.doe@coder.com", roles: [ @@ -56,14 +56,14 @@ export const Editable: Story = { status: "dormant", }, { - ...MockUser, + ...MockUserOwner, username: "Roger Moore", email: "roger.moore@coder.com", roles: [], status: "suspended", }, { - ...MockUser, + ...MockUserOwner, username: "OIDC User", email: "oidc.user@coder.com", roles: [], diff --git a/site/src/pages/WorkspacePage/Workspace.stories.tsx b/site/src/pages/WorkspacePage/Workspace.stories.tsx index 7d29b02c11cb6..957a651788d2c 100644 --- a/site/src/pages/WorkspacePage/Workspace.stories.tsx +++ b/site/src/pages/WorkspacePage/Workspace.stories.tsx @@ -40,7 +40,7 @@ const meta: Meta = { data: Mocks.MockListeningPortsResponse, }, ], - user: Mocks.MockUser, + user: Mocks.MockUserOwner, }, decorators: [ withAuthProvider, diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx index 48dda92b49503..a67690f929b5f 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx @@ -17,7 +17,7 @@ const meta: Meta = { }, decorators: [withDashboardProvider, withDesktopViewport, withAuthProvider], parameters: { - user: Mocks.MockUser, + user: Mocks.MockUserOwner, }, }; @@ -175,7 +175,7 @@ export const CancelShownForUser: Story = { workspace: Mocks.MockStartingWorkspace, }, parameters: { - user: Mocks.MockUser2, + user: Mocks.MockUserMember, }, }; @@ -187,7 +187,7 @@ export const CancelHiddenForUser: Story = { }, }, parameters: { - user: Mocks.MockUser2, + user: Mocks.MockUserMember, }, }; diff --git a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx index 71654b62a5f9a..a9f78f3610983 100644 --- a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx +++ b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx @@ -22,7 +22,7 @@ import { MockTemplate, MockTemplateVersionParameter1, MockTemplateVersionParameter2, - MockUser, + MockUserOwner, MockWorkspace, MockWorkspaceBuild, MockWorkspaceBuildDelete, @@ -320,7 +320,7 @@ describe("WorkspacePage", () => { }); it("restart the workspace with one time parameters when having the confirmation dialog", async () => { - localStorage.removeItem(`${MockUser.id}_ignoredWarnings`); + localStorage.removeItem(`${MockUserOwner.id}_ignoredWarnings`); jest.spyOn(API, "getWorkspaceParameters").mockResolvedValue({ templateVersionRichParameters: [ { diff --git a/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx b/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx index 84af8a518acd8..d9b093513ed8f 100644 --- a/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx @@ -7,7 +7,7 @@ import { MockOrganization, MockTemplate, MockTemplateVersion, - MockUser, + MockUserOwner, MockWorkspace, } from "testHelpers/entities"; import { withAuthProvider, withDashboardProvider } from "testHelpers/storybook"; @@ -41,7 +41,7 @@ const meta: Meta = { chromatic: { diffThreshold: 0.6, }, - user: MockUser, + user: MockUserOwner, }, }; @@ -278,7 +278,7 @@ export const WithQuotaNoOrgs: Story = { { key: getWorkspaceQuotaQueryKey( MockOrganization.name, - MockUser.username, + MockUserOwner.username, ), data: { credits_consumed: 2, @@ -296,7 +296,7 @@ export const WithQuotaWithOrgs: Story = { { key: getWorkspaceQuotaQueryKey( MockOrganization.name, - MockUser.username, + MockUserOwner.username, ), data: { credits_consumed: 2, diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.test.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.test.tsx index 72ff8e165e6a8..9ebede41abe60 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.test.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.test.tsx @@ -1,7 +1,7 @@ import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { http, HttpResponse } from "msw"; -import { MockUser, MockWorkspace } from "testHelpers/entities"; +import { MockUserOwner, MockWorkspace } from "testHelpers/entities"; import { renderWithWorkspaceSettingsLayout } from "testHelpers/renderHelpers"; import { server } from "testHelpers/server"; import { @@ -258,7 +258,7 @@ describe("WorkspaceSchedulePage", () => { }), ); renderWithWorkspaceSettingsLayout(, { - route: `/@${MockUser.username}/${MockWorkspace.name}/schedule`, + route: `/@${MockUserOwner.username}/${MockWorkspace.name}/schedule`, path: "/:username/:workspace/schedule", }); const user = userEvent.setup(); @@ -279,7 +279,7 @@ describe("WorkspaceSchedulePage", () => { describe("autostop change dialog", () => { it("shows if autostop is changed", async () => { renderWithWorkspaceSettingsLayout(, { - route: `/@${MockUser.username}/${MockWorkspace.name}/schedule`, + route: `/@${MockUserOwner.username}/${MockWorkspace.name}/schedule`, path: "/:username/:workspace/schedule", }); const user = userEvent.setup(); @@ -303,7 +303,7 @@ describe("WorkspaceSchedulePage", () => { it("doesn't show if autostop is not changed", async () => { renderWithWorkspaceSettingsLayout(, { - route: `/@${MockUser.username}/${MockWorkspace.name}/schedule`, + route: `/@${MockUserOwner.username}/${MockWorkspace.name}/schedule`, path: "/:username/:workspace/schedule", extraRoutes: [ { path: "/:username/:workspace", element:
Workspace
}, diff --git a/site/src/pages/WorkspacesPage/BatchDeleteConfirmation.stories.tsx b/site/src/pages/WorkspacesPage/BatchDeleteConfirmation.stories.tsx index 1e38068f5bed3..3abb069f05d7b 100644 --- a/site/src/pages/WorkspacesPage/BatchDeleteConfirmation.stories.tsx +++ b/site/src/pages/WorkspacesPage/BatchDeleteConfirmation.stories.tsx @@ -1,7 +1,7 @@ import { action } from "@storybook/addon-actions"; import type { Meta, StoryObj } from "@storybook/react"; import { chromatic } from "testHelpers/chromatic"; -import { MockUser2, MockWorkspace } from "testHelpers/entities"; +import { MockUserMember, MockWorkspace } from "testHelpers/entities"; import { BatchDeleteConfirmation } from "./BatchDeleteConfirmation"; const meta: Meta = { @@ -18,15 +18,15 @@ const meta: Meta = { ...MockWorkspace, name: "Test-Workspace-2", last_used_at: "2023-08-16T15:29:10.302441433Z", - owner_id: MockUser2.id, - owner_name: MockUser2.username, + owner_id: MockUserMember.id, + owner_name: MockUserMember.username, }, { ...MockWorkspace, name: "Test-Workspace-3", last_used_at: "2023-11-16T15:29:10.302441433Z", - owner_id: MockUser2.id, - owner_name: MockUser2.username, + owner_id: MockUserMember.id, + owner_name: MockUserMember.username, }, ], }, diff --git a/site/src/pages/WorkspacesPage/BatchUpdateConfirmation.stories.tsx b/site/src/pages/WorkspacesPage/BatchUpdateConfirmation.stories.tsx index 9a41bf43ff93e..76e8637ece71a 100644 --- a/site/src/pages/WorkspacesPage/BatchUpdateConfirmation.stories.tsx +++ b/site/src/pages/WorkspacesPage/BatchUpdateConfirmation.stories.tsx @@ -8,7 +8,7 @@ import { MockOutdatedWorkspace, MockRunningOutdatedWorkspace, MockTemplateVersion, - MockUser2, + MockUserMember, MockWorkspace, } from "testHelpers/entities"; import { @@ -25,8 +25,8 @@ const workspaces = [ { ...MockRunningOutdatedWorkspace, id: "6", - owner_id: MockUser2.id, - owner_name: MockUser2.username, + owner_id: MockUserMember.id, + owner_name: MockUserMember.username, }, ]; diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx index 47faef89dea0d..f9dc50d0cbff3 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx @@ -21,7 +21,7 @@ import { MockProxyLatencies, MockStoppedWorkspace, MockTemplate, - MockUser, + MockUserOwner, MockWorkspace, MockWorkspaceAppStatus, mockApiError, @@ -105,7 +105,7 @@ const defaultFilterProps = getDefaultFilterProps({ organizations: MockMenu, }, values: { - owner: MockUser.username, + owner: MockUserOwner.username, template: undefined, status: undefined, }, @@ -144,7 +144,7 @@ const meta: Meta = { data: MockBuildInfo, }, ], - user: MockUser, + user: MockUserOwner, }, decorators: [ withAuthProvider, diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index adec32f504d6d..8562a19236da1 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -484,7 +484,7 @@ export const MockMemberPermissions = { viewAuditLog: false, }; -export const MockUser: TypesGen.User = { +export const MockUserOwner: TypesGen.User = { id: "test-user", username: "TestUser", email: "test@coder.com", @@ -499,12 +499,7 @@ export const MockUser: TypesGen.User = { name: "", }; -const MockUserAdmin: TypesGen.User = { - ...MockUser, - roles: [MockUserAdminRole], -}; - -export const MockUser2: TypesGen.User = { +export const MockUserMember: TypesGen.User = { id: "test-user-2", username: "TestUser2", email: "test2@coder.com", @@ -541,28 +536,28 @@ export const MockUserAppearanceSettings: TypesGen.UserAppearanceSettings = { export const MockOrganizationMember: TypesGen.OrganizationMemberWithUserData = { organization_id: MockOrganization.id, - user_id: MockUser.id, - username: MockUser.username, - email: MockUser.email, + user_id: MockUserOwner.id, + username: MockUserOwner.username, + email: MockUserOwner.email, created_at: "", updated_at: "", - name: MockUser.name, - avatar_url: MockUser.avatar_url, - global_roles: MockUser.roles, + name: MockUserOwner.name, + avatar_url: MockUserOwner.avatar_url, + global_roles: MockUserOwner.roles, roles: [], }; export const MockOrganizationMember2: TypesGen.OrganizationMemberWithUserData = { organization_id: MockOrganization.id, - user_id: MockUser2.id, - username: MockUser2.username, - email: MockUser2.email, + user_id: MockUserMember.id, + username: MockUserMember.username, + email: MockUserMember.email, created_at: "", updated_at: "", - name: MockUser2.name, - avatar_url: MockUser2.avatar_url, - global_roles: MockUser2.roles, + name: MockUserMember.name, + avatar_url: MockUserMember.avatar_url, + global_roles: MockUserMember.roles, roles: [], }; @@ -613,7 +608,7 @@ const MockUserAuthProvisioner: TypesGen.ProvisionerDaemon = { ...MockProvisioner, id: "test-user-auth-provisioner", key_id: MockProvisionerUserAuthKey.id, - name: `${MockUser.name}'s provisioner`, + name: `${MockUserOwner.name}'s provisioner`, tags: { scope: "user" }, }; @@ -732,7 +727,7 @@ name:Template test You can add instructions here [Some link info](https://coder.com)`, - created_by: MockUser, + created_by: MockUserOwner, archived: false, }; @@ -751,7 +746,7 @@ name:Template test 2 You can add instructions here [Some link info](https://coder.com)`, - created_by: MockUser, + created_by: MockUserOwner, archived: false, }; @@ -1246,17 +1241,17 @@ export const MockWorkspaceBuild: TypesGen.WorkspaceBuild = { build_number: 1, created_at: "2022-05-17T17:39:01.382927298Z", id: "1", - initiator_id: MockUser.id, - initiator_name: MockUser.username, + initiator_id: MockUserOwner.id, + initiator_name: MockUserOwner.username, job: MockProvisionerJob, template_version_id: MockTemplateVersion.id, template_version_name: MockTemplateVersion.name, transition: "start", updated_at: "2022-05-17T17:39:01.382927298Z", workspace_name: "test-workspace", - workspace_owner_id: MockUser.id, - workspace_owner_name: MockUser.username, - workspace_owner_avatar_url: MockUser.avatar_url, + workspace_owner_id: MockUserOwner.id, + workspace_owner_name: MockUserOwner.username, + workspace_owner_avatar_url: MockUserOwner.avatar_url, workspace_id: "759f1d46-3174-453d-aa60-980a9c1442f3", deadline: "2022-05-17T23:39:00.00Z", reason: "initiator", @@ -1274,17 +1269,17 @@ const MockWorkspaceBuildAutostart: TypesGen.WorkspaceBuild = { build_number: 1, created_at: "2022-05-17T17:39:01.382927298Z", id: "1", - initiator_id: MockUser.id, - initiator_name: MockUser.username, + initiator_id: MockUserOwner.id, + initiator_name: MockUserOwner.username, job: MockProvisionerJob, template_version_id: MockTemplateVersion.id, template_version_name: MockTemplateVersion.name, transition: "start", updated_at: "2022-05-17T17:39:01.382927298Z", workspace_name: "test-workspace", - workspace_owner_id: MockUser.id, - workspace_owner_name: MockUser.username, - workspace_owner_avatar_url: MockUser.avatar_url, + workspace_owner_id: MockUserOwner.id, + workspace_owner_name: MockUserOwner.username, + workspace_owner_avatar_url: MockUserOwner.avatar_url, workspace_id: "759f1d46-3174-453d-aa60-980a9c1442f3", deadline: "2022-05-17T23:39:00.00Z", reason: "autostart", @@ -1298,17 +1293,17 @@ const MockWorkspaceBuildAutostop: TypesGen.WorkspaceBuild = { build_number: 1, created_at: "2022-05-17T17:39:01.382927298Z", id: "1", - initiator_id: MockUser.id, - initiator_name: MockUser.username, + initiator_id: MockUserOwner.id, + initiator_name: MockUserOwner.username, job: MockProvisionerJob, template_version_id: MockTemplateVersion.id, template_version_name: MockTemplateVersion.name, transition: "start", updated_at: "2022-05-17T17:39:01.382927298Z", workspace_name: "test-workspace", - workspace_owner_id: MockUser.id, - workspace_owner_name: MockUser.username, - workspace_owner_avatar_url: MockUser.avatar_url, + workspace_owner_id: MockUserOwner.id, + workspace_owner_name: MockUserOwner.username, + workspace_owner_avatar_url: MockUserOwner.avatar_url, workspace_id: "759f1d46-3174-453d-aa60-980a9c1442f3", deadline: "2022-05-17T23:39:00.00Z", reason: "autostop", @@ -1324,17 +1319,17 @@ export const MockFailedWorkspaceBuild = ( build_number: 1, created_at: "2022-05-17T17:39:01.382927298Z", id: "1", - initiator_id: MockUser.id, - initiator_name: MockUser.username, + initiator_id: MockUserOwner.id, + initiator_name: MockUserOwner.username, job: MockFailedProvisionerJob, template_version_id: MockTemplateVersion.id, template_version_name: MockTemplateVersion.name, transition: transition, updated_at: "2022-05-17T17:39:01.382927298Z", workspace_name: "test-workspace", - workspace_owner_id: MockUser.id, - workspace_owner_name: MockUser.username, - workspace_owner_avatar_url: MockUser.avatar_url, + workspace_owner_id: MockUserOwner.id, + workspace_owner_name: MockUserOwner.username, + workspace_owner_avatar_url: MockUserOwner.avatar_url, workspace_id: "759f1d46-3174-453d-aa60-980a9c1442f3", deadline: "2022-05-17T23:39:00.00Z", reason: "initiator", @@ -1378,10 +1373,10 @@ export const MockWorkspace: TypesGen.Workspace = { template_active_version_id: MockTemplate.active_version_id, template_require_active_version: MockTemplate.require_active_version, outdated: false, - owner_id: MockUser.id, + owner_id: MockUserOwner.id, organization_id: MockOrganization.id, organization_name: "default", - owner_name: MockUser.username, + owner_name: MockUserOwner.username, owner_avatar_url: "https://avatars.githubusercontent.com/u/7122116?v=4", autostart_schedule: MockWorkspaceAutostartEnabled.schedule, ttl_ms: 2 * 60 * 60 * 1000, @@ -2508,7 +2503,7 @@ export const MockAuditLog: TypesGen.AuditLog = { status_code: 200, additional_fields: {}, description: "{user} created workspace {target}", - user: MockUser, + user: MockUserOwner, resource_link: "/@admin/bruno-dev", is_deleted: false, }; @@ -2579,7 +2574,7 @@ export const MockAuditLog3: TypesGen.AuditLog = { status_code: 200, additional_fields: {}, description: "{user} updated template {target}", - user: MockUser, + user: MockUserOwner, resource_link: "/templates/docker", is_deleted: false, }; @@ -2785,7 +2780,7 @@ export const MockGroup: TypesGen.Group = { organization_id: MockOrganization.id, organization_name: MockOrganization.name, organization_display_name: MockOrganization.display_name, - members: [MockUser, MockUser2], + members: [MockUserOwner, MockUserMember], quota_allowance: 5, source: "user", total_member_count: 2, @@ -2799,7 +2794,7 @@ export const MockGroup2: TypesGen.Group = { organization_id: MockOrganization.id, organization_name: MockOrganization.name, organization_display_name: MockOrganization.display_name, - members: [MockUser, MockUser2], + members: [MockUserOwner, MockUserMember], quota_allowance: 5, source: "user", total_member_count: 2, @@ -2826,7 +2821,7 @@ export const MockTemplateACL: TypesGen.TemplateACL = { { ...MockEveryoneGroup, role: "use" }, { ...MockGroup, role: "admin" }, ], - users: [{ ...MockUser, role: "use" }], + users: [{ ...MockUserOwner, role: "use" }], }; export const MockTemplateACLEmpty: TypesGen.TemplateACL = { @@ -4277,7 +4272,7 @@ export const MockNotification: TypesGen.InboxNotification = { url: "https://dev.coder.com/templates/coder/coder", }, ], - user_id: MockUser.id, + user_id: MockUserOwner.id, template_id: MockTemplate.id, targets: [], title: "User account created", diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index 51906fae2ad0d..3f163a4d3a0e8 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -135,12 +135,12 @@ export const handlers = [ // users http.get("/api/v2/users", () => { return HttpResponse.json({ - users: [M.MockUser, M.MockUser2, M.SuspendedMockUser], + users: [M.MockUserOwner, M.MockUserMember, M.SuspendedMockUser], count: 26, }); }), http.post("/api/v2/users", () => { - return HttpResponse.json(M.MockUser); + return HttpResponse.json(M.MockUserOwner); }), http.get("/api/v2/users/:userid/login-type", () => { return HttpResponse.json({ @@ -160,7 +160,7 @@ export const handlers = [ return new HttpResponse(null, { status: 200 }); }), http.get("/api/v2/users/me", () => { - return HttpResponse.json(M.MockUser); + return HttpResponse.json(M.MockUserOwner); }), http.get("/api/v2/users/me/appearance", () => { return HttpResponse.json(M.MockUserAppearanceSettings); @@ -198,7 +198,7 @@ export const handlers = [ return new HttpResponse(null, { status: 200 }); }), http.post("/api/v2/users/first", () => { - return HttpResponse.json(M.MockUser); + return HttpResponse.json(M.MockUserOwner); }), // workspaces diff --git a/site/src/testHelpers/renderHelpers.tsx b/site/src/testHelpers/renderHelpers.tsx index eb76b481783da..5c02eb23150ee 100644 --- a/site/src/testHelpers/renderHelpers.tsx +++ b/site/src/testHelpers/renderHelpers.tsx @@ -20,7 +20,7 @@ import { createMemoryRouter, } from "react-router-dom"; import themes, { DEFAULT_THEME } from "theme"; -import { MockUser } from "./entities"; +import { MockUserOwner } from "./entities"; export function createTestQueryClient() { // Helps create one query client for each test case, to make sure that tests @@ -118,7 +118,7 @@ export function renderWithAuth( return { ...renderResult, - user: MockUser, + user: MockUserOwner, }; } @@ -154,7 +154,7 @@ export function renderWithTemplateSettingsLayout( ); return { - user: MockUser, + user: MockUserOwner, ...renderResult, }; } @@ -191,7 +191,7 @@ export function renderWithWorkspaceSettingsLayout( ); return { - user: MockUser, + user: MockUserOwner, ...renderResult, }; } @@ -228,7 +228,7 @@ export function renderWithOrganizationSettingsLayout( ); return { - user: MockUser, + user: MockUserOwner, ...renderResult, }; } From 43414033466baa15615d6adf1bc545390bc5b224 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 8 May 2025 09:42:39 -0300 Subject: [PATCH 15/46] chore: simplify workspaces data fetching (#17703) We've been using an abstraction that was not necessary to fetch workspaces data. I also took sometime to use the new useWorkspaceUpdate hook in the update workspace tooltip that was missing some important steps like confirmation. --- site/src/components/Badge/Badge.tsx | 2 +- site/src/hooks/usePagination.ts | 6 +- .../WorkspaceOutdatedTooltip.stories.tsx | 23 ++-- .../WorkspaceOutdatedTooltip.tsx | 130 +++++++++--------- .../useWorkspacesToBeDeleted.ts | 25 ++-- .../pages/WorkspacesPage/WorkspacesPage.tsx | 18 +-- .../WorkspacesPage/WorkspacesPageView.tsx | 3 - .../pages/WorkspacesPage/WorkspacesTable.tsx | 13 +- site/src/pages/WorkspacesPage/data.ts | 112 --------------- 9 files changed, 103 insertions(+), 229 deletions(-) delete mode 100644 site/src/pages/WorkspacesPage/data.ts diff --git a/site/src/components/Badge/Badge.tsx b/site/src/components/Badge/Badge.tsx index 8995222027ed0..e6b23b8a4dd94 100644 --- a/site/src/components/Badge/Badge.tsx +++ b/site/src/components/Badge/Badge.tsx @@ -19,7 +19,7 @@ const badgeVariants = cva( warning: "border border-solid border-border-warning bg-surface-orange text-content-warning shadow", destructive: - "border border-solid border-border-destructive bg-surface-red text-content-highlight-red shadow", + "border border-solid border-border-destructive bg-surface-red text-highlight-red shadow", }, size: { xs: "text-2xs font-regular h-5 [&_svg]:hidden rounded px-1.5", diff --git a/site/src/hooks/usePagination.ts b/site/src/hooks/usePagination.ts index 2ab409e85c9d8..72ea70868fb30 100644 --- a/site/src/hooks/usePagination.ts +++ b/site/src/hooks/usePagination.ts @@ -9,7 +9,7 @@ export const usePagination = ({ const [searchParams, setSearchParams] = searchParamsResult; const page = searchParams.get("page") ? Number(searchParams.get("page")) : 1; const limit = DEFAULT_RECORDS_PER_PAGE; - const offset = calcOffset(page, limit); + const offset = page <= 0 ? 0 : (page - 1) * limit; const goToPage = (page: number) => { searchParams.set("page", page.toString()); @@ -23,7 +23,3 @@ export const usePagination = ({ offset, }; }; - -export const calcOffset = (page: number, limit: number) => { - return page <= 0 ? 0 : (page - 1) * limit; -}; diff --git a/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.stories.tsx b/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.stories.tsx index bb0c93e74c8c6..843f8131b793f 100644 --- a/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.stories.tsx +++ b/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.stories.tsx @@ -1,7 +1,10 @@ -import { action } from "@storybook/addon-actions"; import type { Meta, StoryObj } from "@storybook/react"; import { expect, userEvent, waitFor, within } from "@storybook/test"; -import { MockTemplate, MockTemplateVersion } from "testHelpers/entities"; +import { + MockTemplate, + MockTemplateVersion, + MockWorkspace, +} from "testHelpers/entities"; import { withDashboardProvider } from "testHelpers/storybook"; import { WorkspaceOutdatedTooltip } from "./WorkspaceOutdatedTooltip"; @@ -18,9 +21,11 @@ const meta: Meta = { ], }, args: { - onUpdateVersion: action("onUpdateVersion"), - templateName: MockTemplate.display_name, - latestVersionId: MockTemplateVersion.id, + workspace: { + ...MockWorkspace, + template_name: MockTemplate.display_name, + template_active_version_id: MockTemplateVersion.id, + }, }, }; @@ -29,14 +34,12 @@ type Story = StoryObj; const Example: Story = { play: async ({ canvasElement, step }) => { - const screen = within(canvasElement); + const body = within(canvasElement.ownerDocument.body); await step("activate hover trigger", async () => { - await userEvent.hover(screen.getByRole("button")); + await userEvent.hover(body.getByRole("button")); await waitFor(() => - expect( - screen.getByText(MockTemplateVersion.message), - ).toBeInTheDocument(), + expect(body.getByText(MockTemplateVersion.message)).toBeInTheDocument(), ); }); }, diff --git a/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx b/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx index ada4c63d9a0bb..9615560840c59 100644 --- a/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx +++ b/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx @@ -3,7 +3,10 @@ import InfoIcon from "@mui/icons-material/InfoOutlined"; import RefreshIcon from "@mui/icons-material/Refresh"; import Link from "@mui/material/Link"; import Skeleton from "@mui/material/Skeleton"; +import { getErrorDetail, getErrorMessage } from "api/errors"; import { templateVersion } from "api/queries/templates"; +import type { Workspace } from "api/typesGenerated"; +import { displayError } from "components/GlobalSnackbar/utils"; import { HelpTooltip, HelpTooltipAction, @@ -17,102 +20,99 @@ import { usePopover } from "components/deprecated/Popover/Popover"; import { linkToTemplate, useLinks } from "modules/navigation"; import type { FC } from "react"; import { useQuery } from "react-query"; - -const Language = { - outdatedLabel: "Outdated", - versionTooltipText: - "This workspace version is outdated and a newer version is available.", - updateVersionLabel: "Update", -}; +import { + WorkspaceUpdateDialogs, + useWorkspaceUpdate, +} from "../WorkspaceUpdateDialogs"; interface TooltipProps { - organizationName: string; - templateName: string; - latestVersionId: string; - onUpdateVersion: () => void; - ariaLabel?: string; + workspace: Workspace; } export const WorkspaceOutdatedTooltip: FC = (props) => { return ( - + + Outdated info - ); }; -const WorkspaceOutdatedTooltipContent: FC = ({ - organizationName, - templateName, - latestVersionId, - onUpdateVersion, - ariaLabel, -}) => { +const WorkspaceOutdatedTooltipContent: FC = ({ workspace }) => { const getLink = useLinks(); const theme = useTheme(); const popover = usePopover(); const { data: activeVersion } = useQuery({ - ...templateVersion(latestVersionId), + ...templateVersion(workspace.template_active_version_id), enabled: popover.open, }); + const updateWorkspace = useWorkspaceUpdate({ + workspace, + latestVersion: activeVersion, + onError: (error) => { + displayError( + getErrorMessage(error, "Error updating workspace"), + getErrorDetail(error), + ); + }, + }); const versionLink = `${getLink( - linkToTemplate(organizationName, templateName), + linkToTemplate(workspace.organization_name, workspace.template_name), )}`; return ( - - {Language.outdatedLabel} - {Language.versionTooltipText} + <> + + Outdated + + This workspace version is outdated and a newer version is available. + -
-
-
New version
-
- {activeVersion ? ( - - {activeVersion.name} - - ) : ( - - )} +
+
+
New version
+
+ {activeVersion ? ( + + {activeVersion.name} + + ) : ( + + )} +
-
-
-
Message
-
- {activeVersion ? ( - activeVersion.message || "No message" - ) : ( - - )} +
+
Message
+
+ {activeVersion ? ( + activeVersion.message || "No message" + ) : ( + + )} +
-
- - - {Language.updateVersionLabel} - - - + + + Update + + + + + ); }; diff --git a/site/src/pages/TemplateSettingsPage/TemplateSchedulePage/useWorkspacesToBeDeleted.ts b/site/src/pages/TemplateSettingsPage/TemplateSchedulePage/useWorkspacesToBeDeleted.ts index 5a974d5d8fe31..d55f0b6c54fff 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSchedulePage/useWorkspacesToBeDeleted.ts +++ b/site/src/pages/TemplateSettingsPage/TemplateSchedulePage/useWorkspacesToBeDeleted.ts @@ -1,7 +1,7 @@ +import { workspaces } from "api/queries/workspaces"; import type { Template, Workspace } from "api/typesGenerated"; import { compareAsc } from "date-fns"; -import { calcOffset } from "hooks/usePagination"; -import { useWorkspacesData } from "pages/WorkspacesPage/data"; +import { useQuery } from "react-query"; import type { TemplateScheduleFormValues } from "./formHelpers"; export const useWorkspacesToGoDormant = ( @@ -9,11 +9,11 @@ export const useWorkspacesToGoDormant = ( formValues: TemplateScheduleFormValues, fromDate: Date, ) => { - const { data } = useWorkspacesData({ - offset: calcOffset(0, 0), - limit: 0, - q: `template:${template.name}`, - }); + const { data } = useQuery( + workspaces({ + q: `template:${template.name}`, + }), + ); return data?.workspaces?.filter((workspace: Workspace) => { if (!formValues.time_til_dormant_ms) { @@ -40,11 +40,12 @@ export const useWorkspacesToBeDeleted = ( formValues: TemplateScheduleFormValues, fromDate: Date, ) => { - const { data } = useWorkspacesData({ - offset: calcOffset(0, 0), - limit: 0, - q: `template:${template.name} dormant:true`, - }); + const { data } = useQuery( + workspaces({ + q: `template:${template.name} dormant:true`, + }), + ); + return data?.workspaces?.filter((workspace: Workspace) => { if (!workspace.dormant_at || !formValues.time_til_dormant_autodelete_ms) { return false; diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx index 551c554fd5ee3..d0439be0f0462 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -1,6 +1,7 @@ import { getErrorDetail, getErrorMessage } from "api/errors"; import { workspacePermissionsByOrganization } from "api/queries/organizations"; import { templates } from "api/queries/templates"; +import { workspaces } from "api/queries/workspaces"; import type { Workspace } from "api/typesGenerated"; import { useFilter } from "components/Filter/Filter"; import { useUserFilterMenu } from "components/Filter/UserFilter"; @@ -19,7 +20,6 @@ import { BatchDeleteConfirmation } from "./BatchDeleteConfirmation"; import { BatchUpdateConfirmation } from "./BatchUpdateConfirmation"; import { WorkspacesPageView } from "./WorkspacesPageView"; import { useBatchActions } from "./batchActions"; -import { useWorkspaceUpdate, useWorkspacesData } from "./data"; import { useStatusFilterMenu, useTemplateFilterMenu } from "./filter/menus"; function useSafeSearchParams() { @@ -45,9 +45,7 @@ const WorkspacesPage: FC = () => { const pagination = usePagination({ searchParamsResult }); const { permissions, user: me } = useAuthenticated(); const { entitlements } = useDashboard(); - const templatesQuery = useQuery(templates()); - const workspacePermissionsQuery = useQuery( workspacePermissionsByOrganization( templatesQuery.data?.map((template) => template.organization_id), @@ -73,12 +71,17 @@ const WorkspacesPage: FC = () => { onFilterChange: () => pagination.goToPage(1), }); - const { data, error, queryKey, refetch } = useWorkspacesData({ + const workspacesQueryOptions = workspaces({ ...pagination, q: filterProps.filter.query, }); + const { data, error, refetch } = useQuery({ + ...workspacesQueryOptions, + refetchInterval: (_, query) => { + return query.state.error ? false : 5_000; + }, + }); - const updateWorkspace = useWorkspaceUpdate(queryKey); const [checkedWorkspaces, setCheckedWorkspaces] = useState< readonly Workspace[] >([]); @@ -123,9 +126,6 @@ const WorkspacesPage: FC = () => { limit={pagination.limit} onPageChange={pagination.goToPage} filterProps={filterProps} - onUpdateWorkspace={(workspace) => { - updateWorkspace.mutate(workspace); - }} isRunningBatchAction={batchActions.isLoading} onDeleteAll={() => setConfirmingBatchAction("delete")} onUpdateAll={() => setConfirmingBatchAction("update")} @@ -133,7 +133,7 @@ const WorkspacesPage: FC = () => { onStopAll={() => batchActions.stopAll(checkedWorkspaces)} onActionSuccess={async () => { await queryClient.invalidateQueries({ - queryKey, + queryKey: workspacesQueryOptions.queryKey, }); }} onActionError={(error) => { diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx index 6db41fef8fa6c..6633f884e1263 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx @@ -53,7 +53,6 @@ export interface WorkspacesPageViewProps { page: number; limit: number; onPageChange: (page: number) => void; - onUpdateWorkspace: (workspace: Workspace) => void; onCheckChange: (checkedWorkspaces: readonly Workspace[]) => void; isRunningBatchAction: boolean; onDeleteAll: () => void; @@ -76,7 +75,6 @@ export const WorkspacesPageView: FC = ({ count, filterProps, onPageChange, - onUpdateWorkspace, page, checkedWorkspaces, onCheckChange, @@ -223,7 +221,6 @@ export const WorkspacesPageView: FC = ({ canCreateTemplate={canCreateTemplate} workspaces={workspaces} isUsingFilter={filterProps.filter.used} - onUpdateWorkspace={onUpdateWorkspace} checkedWorkspaces={checkedWorkspaces} onCheckChange={onCheckChange} canCheckWorkspaces={canCheckWorkspaces} diff --git a/site/src/pages/WorkspacesPage/WorkspacesTable.tsx b/site/src/pages/WorkspacesPage/WorkspacesTable.tsx index 92dcee60dec96..b4f1c98b27261 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesTable.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesTable.tsx @@ -97,7 +97,6 @@ export interface WorkspacesTableProps { checkedWorkspaces: readonly Workspace[]; error?: unknown; isUsingFilter: boolean; - onUpdateWorkspace: (workspace: Workspace) => void; onCheckChange: (checkedWorkspaces: readonly Workspace[]) => void; canCheckWorkspaces: boolean; templates?: Template[]; @@ -110,7 +109,6 @@ export const WorkspacesTable: FC = ({ workspaces, checkedWorkspaces, isUsingFilter, - onUpdateWorkspace, onCheckChange, canCheckWorkspaces, templates, @@ -243,16 +241,7 @@ export const WorkspacesTable: FC = ({ {workspace.name} {workspace.favorite && } {workspace.outdated && ( - { - onUpdateWorkspace(workspace); - }} - /> + )} } diff --git a/site/src/pages/WorkspacesPage/data.ts b/site/src/pages/WorkspacesPage/data.ts deleted file mode 100644 index 764ea218aa96c..0000000000000 --- a/site/src/pages/WorkspacesPage/data.ts +++ /dev/null @@ -1,112 +0,0 @@ -import { API } from "api/api"; -import { getErrorMessage } from "api/errors"; -import { workspaces } from "api/queries/workspaces"; -import type { - Workspace, - WorkspaceBuild, - WorkspacesRequest, - WorkspacesResponse, -} from "api/typesGenerated"; -import { displayError } from "components/GlobalSnackbar/utils"; -import { useState } from "react"; -import { - type QueryKey, - useMutation, - useQuery, - useQueryClient, -} from "react-query"; - -export const useWorkspacesData = (req: WorkspacesRequest) => { - const [shouldRefetch, setShouldRefetch] = useState(true); - const workspacesQueryOptions = workspaces(req); - const result = useQuery({ - ...workspacesQueryOptions, - onSuccess: () => { - setShouldRefetch(true); - }, - onError: () => { - setShouldRefetch(false); - }, - refetchInterval: shouldRefetch ? 5_000 : undefined, - }); - - return { - ...result, - queryKey: workspacesQueryOptions.queryKey, - }; -}; - -export const useWorkspaceUpdate = (queryKey: QueryKey) => { - const queryClient = useQueryClient(); - - return useMutation({ - mutationFn: API.updateWorkspaceVersion, - onMutate: async (workspace) => { - await queryClient.cancelQueries({ queryKey }); - queryClient.setQueryData(queryKey, (oldResponse) => { - if (oldResponse) { - return assignPendingStatus(oldResponse, workspace); - } - }); - }, - onSuccess: (workspaceBuild) => { - queryClient.setQueryData(queryKey, (oldResponse) => { - if (oldResponse) { - return assignLatestBuild(oldResponse, workspaceBuild); - } - }); - }, - onError: (error) => { - const message = getErrorMessage( - error, - "Error updating workspace version", - ); - displayError(message); - }, - }); -}; - -const assignLatestBuild = ( - oldResponse: WorkspacesResponse, - build: WorkspaceBuild, -): WorkspacesResponse => { - return { - ...oldResponse, - workspaces: oldResponse.workspaces.map((workspace) => { - if (workspace.id === build.workspace_id) { - return { - ...workspace, - latest_build: build, - }; - } - - return workspace; - }), - }; -}; - -const assignPendingStatus = ( - oldResponse: WorkspacesResponse, - workspace: Workspace, -): WorkspacesResponse => { - return { - ...oldResponse, - workspaces: oldResponse.workspaces.map((workspaceItem) => { - if (workspaceItem.id === workspace.id) { - return { - ...workspace, - latest_build: { - ...workspace.latest_build, - status: "pending", - job: { - ...workspace.latest_build.job, - status: "pending", - }, - }, - } as Workspace; - } - - return workspaceItem; - }), - }; -}; From 857587b35d3eaff2f8a1062c9e4cfb66b786616b Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 8 May 2025 09:51:10 -0300 Subject: [PATCH 16/46] fix: do not share token with http app urls (#17720) It's a security issue to share the API token, and the protocols that we actually want to share it with are not HTTP and handled locally on the same machine. Security issue introduced by https://github.com/coder/coder/pull/17708 --- site/src/modules/resources/AppLink/AppLink.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/site/src/modules/resources/AppLink/AppLink.tsx b/site/src/modules/resources/AppLink/AppLink.tsx index 5c4209a8f72c7..0e94335ba0c43 100644 --- a/site/src/modules/resources/AppLink/AppLink.tsx +++ b/site/src/modules/resources/AppLink/AppLink.tsx @@ -106,7 +106,11 @@ export const AppLink: FC = ({ app, workspace, agent }) => { event.preventDefault(); - if (app.external) { + // HTTP links should never need the session token, since Cookies + // handle sharing it when you access the Coder Dashboard. We should + // never be forwarding the bare session token to other domains! + const isHttp = app.url?.startsWith("http"); + if (app.external && !isHttp) { // This is a magic undocumented string that is replaced // with a brand-new session token from the backend. // This only exists for external URLs, and should only From 1bb96b85287c778fb5c12a5c8547c1ca1629a1ea Mon Sep 17 00:00:00 2001 From: Vincent Vielle Date: Thu, 8 May 2025 15:49:57 +0200 Subject: [PATCH 17/46] fix: resolve flake test on manager (#17702) Fixes coder/internal#544 --------- Co-authored-by: Mathias Fredriksson --- coderd/notifications/manager.go | 110 +++++++++++++-------------- coderd/notifications/manager_test.go | 22 ++++++ 2 files changed, 74 insertions(+), 58 deletions(-) diff --git a/coderd/notifications/manager.go b/coderd/notifications/manager.go index ee85bd2d7a3c4..1a2c418a014bb 100644 --- a/coderd/notifications/manager.go +++ b/coderd/notifications/manager.go @@ -44,7 +44,6 @@ type Manager struct { store Store log slog.Logger - notifier *notifier handlers map[database.NotificationMethod]Handler method database.NotificationMethod helpers template.FuncMap @@ -53,11 +52,13 @@ type Manager struct { success, failure chan dispatchResult - runOnce sync.Once - stopOnce sync.Once - doneOnce sync.Once - stop chan any - done chan any + mu sync.Mutex // Protects following. + closed bool + notifier *notifier + + runOnce sync.Once + stop chan any + done chan any // clock is for testing only clock quartz.Clock @@ -138,7 +139,7 @@ func (m *Manager) WithHandlers(reg map[database.NotificationMethod]Handler) { // Manager requires system-level permissions to interact with the store. // Run is only intended to be run once. func (m *Manager) Run(ctx context.Context) { - m.log.Info(ctx, "started") + m.log.Debug(ctx, "notification manager started") m.runOnce.Do(func() { // Closes when Stop() is called or context is canceled. @@ -155,31 +156,26 @@ func (m *Manager) Run(ctx context.Context) { // events, creating a notifier, and publishing bulk dispatch result updates to the store. func (m *Manager) loop(ctx context.Context) error { defer func() { - m.doneOnce.Do(func() { - close(m.done) - }) - m.log.Info(context.Background(), "notification manager stopped") + close(m.done) + m.log.Debug(context.Background(), "notification manager stopped") }() - // Caught a terminal signal before notifier was created, exit immediately. - select { - case <-m.stop: - m.log.Warn(ctx, "gracefully stopped") - return xerrors.Errorf("gracefully stopped") - case <-ctx.Done(): - m.log.Error(ctx, "ungracefully stopped", slog.Error(ctx.Err())) - return xerrors.Errorf("notifications: %w", ctx.Err()) - default: + m.mu.Lock() + if m.closed { + m.mu.Unlock() + return xerrors.New("manager already closed") } var eg errgroup.Group - // Create a notifier to run concurrently, which will handle dequeueing and dispatching notifications. m.notifier = newNotifier(ctx, m.cfg, uuid.New(), m.log, m.store, m.handlers, m.helpers, m.metrics, m.clock) eg.Go(func() error { + // run the notifier which will handle dequeueing and dispatching notifications. return m.notifier.run(m.success, m.failure) }) + m.mu.Unlock() + // Periodically flush notification state changes to the store. eg.Go(func() error { // Every interval, collect the messages in the channels and bulk update them in the store. @@ -355,48 +351,46 @@ func (m *Manager) syncUpdates(ctx context.Context) { // Stop stops the notifier and waits until it has stopped. func (m *Manager) Stop(ctx context.Context) error { - var err error - m.stopOnce.Do(func() { - select { - case <-ctx.Done(): - err = ctx.Err() - return - default: - } + m.mu.Lock() + defer m.mu.Unlock() - m.log.Info(context.Background(), "graceful stop requested") + if m.closed { + return nil + } + m.closed = true - // If the notifier hasn't been started, we don't need to wait for anything. - // This is only really during testing when we want to enqueue messages only but not deliver them. - if m.notifier == nil { - m.doneOnce.Do(func() { - close(m.done) - }) - } else { - m.notifier.stop() - } + m.log.Debug(context.Background(), "graceful stop requested") + + // If the notifier hasn't been started, we don't need to wait for anything. + // This is only really during testing when we want to enqueue messages only but not deliver them. + if m.notifier != nil { + m.notifier.stop() + } - // Signal the stop channel to cause loop to exit. - close(m.stop) + // Signal the stop channel to cause loop to exit. + close(m.stop) - // Wait for the manager loop to exit or the context to be canceled, whichever comes first. - select { - case <-ctx.Done(): - var errStr string - if ctx.Err() != nil { - errStr = ctx.Err().Error() - } - // For some reason, slog.Error returns {} for a context error. - m.log.Error(context.Background(), "graceful stop failed", slog.F("err", errStr)) - err = ctx.Err() - return - case <-m.done: - m.log.Info(context.Background(), "gracefully stopped") - return - } - }) + if m.notifier == nil { + return nil + } - return err + m.mu.Unlock() // Unlock to avoid blocking loop. + defer m.mu.Lock() // Re-lock the mutex due to earlier defer. + + // Wait for the manager loop to exit or the context to be canceled, whichever comes first. + select { + case <-ctx.Done(): + var errStr string + if ctx.Err() != nil { + errStr = ctx.Err().Error() + } + // For some reason, slog.Error returns {} for a context error. + m.log.Error(context.Background(), "graceful stop failed", slog.F("err", errStr)) + return ctx.Err() + case <-m.done: + m.log.Debug(context.Background(), "gracefully stopped") + return nil + } } type dispatchResult struct { diff --git a/coderd/notifications/manager_test.go b/coderd/notifications/manager_test.go index 3eaebef7c9d0f..e9c309f0a09d3 100644 --- a/coderd/notifications/manager_test.go +++ b/coderd/notifications/manager_test.go @@ -182,6 +182,28 @@ func TestStopBeforeRun(t *testing.T) { }, testutil.WaitShort, testutil.IntervalFast) } +func TestRunStopRace(t *testing.T) { + t.Parallel() + + // SETUP + + // nolint:gocritic // Unit test. + ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitMedium)) + store, ps := dbtestutil.NewDB(t) + logger := testutil.Logger(t) + + // GIVEN: a standard manager + mgr, err := notifications.NewManager(defaultNotificationsConfig(database.NotificationMethodSmtp), store, ps, defaultHelpers(), createMetrics(), logger.Named("notifications-manager")) + require.NoError(t, err) + + // Start Run and Stop after each other (run does "go loop()"). + // This is to catch a (now fixed) race condition where the manager + // would be accessed/stopped while it was being created/starting up. + mgr.Run(ctx) + err = mgr.Stop(ctx) + require.NoError(t, err) +} + type syncInterceptor struct { notifications.Store From c5c3a54fcaaed37a979056a859d518ea204334dd Mon Sep 17 00:00:00 2001 From: brettkolodny Date: Thu, 8 May 2025 10:10:52 -0400 Subject: [PATCH 18/46] fix: create ssh directory if it doesn't already exist when running `coder config-ssh` (#17711) Closes [coder/internal#623](https://github.com/coder/internal/issues/623) > [!WARNING] > PR co-authored by Claude Code --- cli/configssh.go | 5 +++++ cli/configssh_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/cli/configssh.go b/cli/configssh.go index 65f36697d873f..e3e168d2b198c 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -440,6 +440,11 @@ func (r *RootCmd) configSSH() *serpent.Command { } if !bytes.Equal(configRaw, configModified) { + sshDir := filepath.Dir(sshConfigFile) + if err := os.MkdirAll(sshDir, 0700); err != nil { + return xerrors.Errorf("failed to create directory %q: %w", sshDir, err) + } + err = atomic.WriteFile(sshConfigFile, bytes.NewReader(configModified)) if err != nil { return xerrors.Errorf("write ssh config failed: %w", err) diff --git a/cli/configssh_test.go b/cli/configssh_test.go index 72faaa00c1ca0..60c93b8e94f4b 100644 --- a/cli/configssh_test.go +++ b/cli/configssh_test.go @@ -169,6 +169,47 @@ func TestConfigSSH(t *testing.T) { <-copyDone } +func TestConfigSSH_MissingDirectory(t *testing.T) { + t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("See coder/internal#117") + } + + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + + // Create a temporary directory but don't create .ssh subdirectory + tmpdir := t.TempDir() + sshConfigPath := filepath.Join(tmpdir, ".ssh", "config") + + // Run config-ssh with a non-existent .ssh directory + args := []string{ + "config-ssh", + "--ssh-config-file", sshConfigPath, + "--yes", // Skip confirmation prompts + } + inv, root := clitest.New(t, args...) + clitest.SetupConfig(t, client, root) + + err := inv.Run() + require.NoError(t, err, "config-ssh should succeed with non-existent directory") + + // Verify that the .ssh directory was created + sshDir := filepath.Dir(sshConfigPath) + _, err = os.Stat(sshDir) + require.NoError(t, err, ".ssh directory should exist") + + // Verify that the config file was created + _, err = os.Stat(sshConfigPath) + require.NoError(t, err, "config file should exist") + + // Check that the directory has proper permissions (0700) + sshDirInfo, err := os.Stat(sshDir) + require.NoError(t, err) + require.Equal(t, os.FileMode(0700), sshDirInfo.Mode().Perm(), "directory should have 0700 permissions") +} + func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { t.Parallel() From 0b141c47cb3ed9faebc7c44798a9324569e9e4bb Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 8 May 2025 15:12:05 +0100 Subject: [PATCH 19/46] chore: add DynamicParameter stories (#17710) resolves coder/preview#112 - Add stories for DynamicParameter component - fix bug with displaying immutable badge and required asterisk --- .../DynamicParameter.stories.tsx | 197 ++++++++++++++++++ .../DynamicParameter/DynamicParameter.tsx | 4 +- site/src/testHelpers/entities.ts | 19 ++ 3 files changed, 218 insertions(+), 2 deletions(-) create mode 100644 site/src/modules/workspaces/DynamicParameter/DynamicParameter.stories.tsx diff --git a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.stories.tsx b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.stories.tsx new file mode 100644 index 0000000000000..03aef9e6363bf --- /dev/null +++ b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.stories.tsx @@ -0,0 +1,197 @@ +import type { Meta, StoryObj } from "@storybook/react"; +import { MockPreviewParameter } from "testHelpers/entities"; +import { DynamicParameter } from "./DynamicParameter"; + +const meta: Meta = { + title: "modules/workspaces/DynamicParameter", + component: DynamicParameter, + parameters: { + layout: "centered", + }, +}; + +export default meta; +type Story = StoryObj; + +export const TextInput: Story = { + args: { + parameter: { + ...MockPreviewParameter, + }, + }, +}; + +export const TextArea: Story = { + args: { + parameter: { + ...MockPreviewParameter, + form_type: "textarea", + }, + }, +}; + +export const Checkbox: Story = { + args: { + parameter: { + ...MockPreviewParameter, + form_type: "checkbox", + type: "bool", + }, + }, +}; + +export const Switch: Story = { + args: { + parameter: { + ...MockPreviewParameter, + form_type: "switch", + type: "bool", + }, + }, +}; + +export const Dropdown: Story = { + args: { + parameter: { + ...MockPreviewParameter, + form_type: "dropdown", + type: "string", + options: [ + { + name: "Option 1", + value: { valid: true, value: "option1" }, + description: "this is option 1", + icon: "", + }, + { + name: "Option 2", + value: { valid: true, value: "option2" }, + description: "this is option 2", + icon: "", + }, + { + name: "Option 3", + value: { valid: true, value: "option3" }, + description: "this is option 3", + icon: "", + }, + ], + }, + }, +}; + +export const MultiSelect: Story = { + args: { + parameter: { + ...MockPreviewParameter, + form_type: "multi-select", + type: "list(string)", + options: [ + { + name: "Red", + value: { valid: true, value: "red" }, + description: "this is red", + icon: "", + }, + { + name: "Green", + value: { valid: true, value: "green" }, + description: "this is green", + icon: "", + }, + { + name: "Blue", + value: { valid: true, value: "blue" }, + description: "this is blue", + icon: "", + }, + { + name: "Purple", + value: { valid: true, value: "purple" }, + description: "this is purple", + icon: "", + }, + ], + }, + }, +}; + +export const Radio: Story = { + args: { + parameter: { + ...MockPreviewParameter, + form_type: "radio", + type: "string", + options: [ + { + name: "Small", + value: { valid: true, value: "small" }, + description: "this is small", + icon: "", + }, + { + name: "Medium", + value: { valid: true, value: "medium" }, + description: "this is medium", + icon: "", + }, + { + name: "Large", + value: { valid: true, value: "large" }, + description: "this is large", + icon: "", + }, + ], + }, + }, +}; + +export const Slider: Story = { + args: { + parameter: { + ...MockPreviewParameter, + form_type: "slider", + type: "number", + }, + }, +}; + +export const Disabled: Story = { + args: { + parameter: { + ...MockPreviewParameter, + value: { valid: true, value: "disabled value" }, + }, + disabled: true, + }, +}; + +export const Preset: Story = { + args: { + parameter: { + ...MockPreviewParameter, + value: { valid: true, value: "preset value" }, + }, + isPreset: true, + }, +}; + +export const Immutable: Story = { + args: { + parameter: { + ...MockPreviewParameter, + mutable: false, + }, + }, +}; + +export const AllBadges: Story = { + args: { + parameter: { + ...MockPreviewParameter, + value: { valid: true, value: "us-west-2" }, + mutable: false, + }, + isPreset: true, + }, +}; diff --git a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx index 5f8e875dbebcf..8870602f7dcd1 100644 --- a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx +++ b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx @@ -97,11 +97,11 @@ const ParameterLabel: FC = ({ parameter, isPreset }) => {