From 730c769ab303569bd99c1dba93a595e6c1d251cb Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 29 May 2026 14:38:33 +0000 Subject: [PATCH] fix(cli): address chat share add review feedback --- cli/exp_chat_share.go | 13 ++-- cli/exp_chat_share_add_test.go | 23 ++++++ enterprise/cli/exp_chat_share_add_test.go | 88 +++++++++++++++++++++++ 3 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 enterprise/cli/exp_chat_share_add_test.go diff --git a/cli/exp_chat_share.go b/cli/exp_chat_share.go index 9029c6036e..fd4eec9a95 100644 --- a/cli/exp_chat_share.go +++ b/cli/exp_chat_share.go @@ -29,11 +29,10 @@ func (r *RootCmd) chatShareCommand() *serpent.Command { const chatShareDefaultGroupDisplay = "-" type chatRoleLookupParams struct { - Client *codersdk.Client - OrgID uuid.UUID - OrgName string - Users [][2]string - Groups [][2]string + Client *codersdk.Client + OrgID uuid.UUID + Users [][2]string + Groups [][2]string } func parseChatShareID(raw string) (uuid.UUID, error) { @@ -86,7 +85,7 @@ func fetchChatUsersAndGroups(ctx context.Context, params chatRoleLookupParams) ( } } if userID == "" { - return nil, nil, xerrors.Errorf("could not find user %s in the organization %s", username, params.OrgName) + return nil, nil, xerrors.Errorf("could not find user %s in the organization %s", username, params.OrgID.String()) } chatRole, err := stringToChatRole(role) @@ -118,7 +117,7 @@ func fetchChatUsersAndGroups(ctx context.Context, params chatRoleLookupParams) ( } } if orgGroup == nil { - return nil, nil, xerrors.Errorf("could not find group named %s belonging to the organization %s", groupName, params.OrgName) + return nil, nil, xerrors.Errorf("could not find group named %s belonging to the organization %s", groupName, params.OrgID.String()) } chatRole, err := stringToChatRole(role) diff --git a/cli/exp_chat_share_add_test.go b/cli/exp_chat_share_add_test.go index 02d5997a15..f9c6f8d9bc 100644 --- a/cli/exp_chat_share_add_test.go +++ b/cli/exp_chat_share_add_test.go @@ -152,6 +152,29 @@ func TestExpChatShareAdd(t *testing.T) { require.Contains(t, err.Error(), "invalid role \"write\"") }) + t.Run("RejectsMissingUserWithOrganizationID", func(t *testing.T) { + t.Parallel() + + client, db := coderdtest.NewWithDatabase(t, nil) + firstUser := coderdtest.CreateFirstUser(t, client) + modelConfig := dbgen.ChatModelConfig(t, db, database.ChatModelConfig{}) + chat := dbgen.Chat(t, db, database.Chat{ + OrganizationID: firstUser.OrganizationID, + OwnerID: firstUser.UserID, + LastModelConfigID: modelConfig.ID, + Title: "share add missing user", + }) + + ctx := testutil.Context(t, testutil.WaitMedium) + inv, root := clitest.New(t, "exp", "chat", "share", "add", chat.ID.String(), "--user", "missing-user:read") + 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) + require.Contains(t, err.Error(), "could not find user missing-user in the organization") + require.Contains(t, err.Error(), firstUser.OrganizationID.String()) + }) + t.Run("RequiresActor", func(t *testing.T) { t.Parallel() diff --git a/enterprise/cli/exp_chat_share_add_test.go b/enterprise/cli/exp_chat_share_add_test.go new file mode 100644 index 0000000000..697b0c594d --- /dev/null +++ b/enterprise/cli/exp_chat_share_add_test.go @@ -0,0 +1,88 @@ +package cli_test + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/cli/clitest" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/testutil" +) + +func TestEnterpriseExpChatShareAdd(t *testing.T) { + t.Parallel() + + t.Run("ShareWithGroupExplicitReadRole", func(t *testing.T) { + t.Parallel() + + client, db, firstUser := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }, + }) + group := coderdtest.CreateGroup(t, client, firstUser.OrganizationID, "chat-share-group") + modelConfig := dbgen.ChatModelConfig(t, db, database.ChatModelConfig{}) + chat := dbgen.Chat(t, db, database.Chat{ + OrganizationID: firstUser.OrganizationID, + OwnerID: firstUser.UserID, + LastModelConfigID: modelConfig.ID, + Title: "share add group", + }) + + ctx := testutil.Context(t, testutil.WaitMedium) + inv, root := newCLI(t, "exp", "chat", "share", "add", chat.ID.String(), "--group", group.Name+":read") + clitest.SetupConfig(t, client, root) + + out := new(bytes.Buffer) + inv.Stdout = out + err := inv.WithContext(ctx).Run() + require.NoError(t, err) + + acl, err := codersdk.NewExperimentalClient(client).GetChatACL(ctx, chat.ID) + require.NoError(t, err) + require.Len(t, acl.Groups, 1) + assert.Equal(t, group.ID, acl.Groups[0].ID) + assert.Equal(t, group.Name, acl.Groups[0].Name) + assert.Equal(t, codersdk.ChatRoleRead, acl.Groups[0].Role) + assert.Contains(t, out.String(), group.Name) + assert.Contains(t, out.String(), string(codersdk.ChatRoleRead)) + }) + + t.Run("RejectsMissingGroupWithOrganizationID", func(t *testing.T) { + t.Parallel() + + client, db, firstUser := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }, + }) + modelConfig := dbgen.ChatModelConfig(t, db, database.ChatModelConfig{}) + chat := dbgen.Chat(t, db, database.Chat{ + OrganizationID: firstUser.OrganizationID, + OwnerID: firstUser.UserID, + LastModelConfigID: modelConfig.ID, + Title: "share add missing group", + }) + + ctx := testutil.Context(t, testutil.WaitMedium) + inv, root := newCLI(t, "exp", "chat", "share", "add", chat.ID.String(), "--group", "missing-group:read") + clitest.SetupConfig(t, client, root) + + err := inv.WithContext(ctx).Run() + require.Error(t, err) + require.Contains(t, err.Error(), "could not find group named missing-group belonging to the organization") + require.Contains(t, err.Error(), firstUser.OrganizationID.String()) + }) +}