From 44bc32149eafdc2f476e8812db266bd73780e41b Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 29 May 2026 10:27:14 +0000 Subject: [PATCH 1/2] refactor(cli): clean up chat share add helpers --- cli/exp_chat_share.go | 11 +++++------ cli/exp_chat_share_add_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/cli/exp_chat_share.go b/cli/exp_chat_share.go index 7a0bd432bb..4abaf2ca38 100644 --- a/cli/exp_chat_share.go +++ b/cli/exp_chat_share.go @@ -46,21 +46,20 @@ func parseChatShareID(raw string) (uuid.UUID, error) { } func parseChatShareActorRole(raw string) ([2]string, error) { - if strings.Count(raw, ":") > 1 { + name, role, hasRole := strings.Cut(raw, ":") + if strings.Contains(role, ":") { return [2]string{}, xerrors.New("must match pattern 'name:role'") } - parts := strings.SplitN(raw, ":", 2) - name := parts[0] if name == "" || !codersdk.UsernameValidRegex.MatchString(name) { return [2]string{}, xerrors.New("invalid name") } - if len(parts) == 1 { + if !hasRole { return [2]string{name, ""}, nil } - if parts[1] == "" { + if role == "" { return [2]string{}, xerrors.New("role cannot be empty") } - return [2]string{name, parts[1]}, nil + return [2]string{name, role}, nil } func stringToChatRole(role string) (codersdk.ChatRole, error) { diff --git a/cli/exp_chat_share_add_test.go b/cli/exp_chat_share_add_test.go index 4c1a3b08f9..36d6685ccb 100644 --- a/cli/exp_chat_share_add_test.go +++ b/cli/exp_chat_share_add_test.go @@ -35,7 +35,7 @@ func TestExpChatShareAdd(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) inv, root := clitest.New(t, "exp", "chat", "share", "add", chat.ID.String(), "--user", toShareWithUser.Username+":read") - clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL updates require the chat owner to fetch and modify the chat. + clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL operations require the chat owner in this fixture. out := new(bytes.Buffer) inv.Stdout = out @@ -73,7 +73,7 @@ func TestExpChatShareAdd(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) inv, root := clitest.New(t, "exp", "chat", "share", "add", chat.ID.String(), "--user", toShareWithUser.Username) - clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL updates require the chat owner to fetch and modify the chat. + clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL operations require the chat owner in this fixture. out := new(bytes.Buffer) inv.Stdout = out @@ -113,7 +113,7 @@ func TestExpChatShareAdd(t *testing.T) { "exp", "chat", "share", "add", chat.ID.String(), fmt.Sprintf("--user=%s:read,%s:read", toShareWithUser1.Username, toShareWithUser2.Username), ) - clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL updates require the chat owner to fetch and modify the chat. + clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL operations require the chat owner in this fixture. out := new(bytes.Buffer) inv.Stdout = out @@ -158,7 +158,7 @@ func TestExpChatShareAdd(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) inv, root := clitest.New(t, "exp", "chat", "share", "add", chat.ID.String(), "--user", toShareWithUser.Username+":write") - clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL updates require the chat owner to fetch and modify the chat. + clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL operations require the chat owner in this fixture. err := inv.WithContext(ctx).Run() require.Error(t, err) From 6c33ef279932e6f580d305a1ff4d1532e30f6633 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 29 May 2026 10:28:41 +0000 Subject: [PATCH 2/2] refactor(cli): clean up chat share remove tests --- cli/exp_chat_share_remove_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/exp_chat_share_remove_test.go b/cli/exp_chat_share_remove_test.go index ad93ff434d..8b013a1ee5 100644 --- a/cli/exp_chat_share_remove_test.go +++ b/cli/exp_chat_share_remove_test.go @@ -39,7 +39,7 @@ func TestExpChatShareRemove(t *testing.T) { require.NoError(t, err) inv, root := clitest.New(t, "exp", "chat", "share", "remove", chat.ID.String(), "--user", sharedUser.Username) - clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL updates require the chat owner to fetch and modify the chat. + clitest.SetupConfig(t, client, root) //nolint:gocritic // Chat ACL operations require the chat owner in this fixture. out := new(bytes.Buffer) inv.Stdout = out