Skip to content

feat: update 'collaborator' command to support read-only permission type #272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mwbrooks merged 11 commits into from
Nov 27, 2025
Merged

feat: update 'collaborator' command to support read-only permission type #272

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
22ed643
unhide read-only collaborator setting for deployed apps
vegeris Nov 20, 2025
ccaf2df
Merge branch 'main' of https://github.com/slackapi/slack-cli into eve…
vegeris Nov 20, 2025
1100c37
update
vegeris Nov 20, 2025
48841e0
update prompt wording
vegeris Nov 20, 2025
236dad9
Merge branch 'main' into evegeris-release-read-only-app-collabs
vegeris Nov 21, 2025
9c46e30
Merge branch 'main' into evegeris-release-read-only-app-collabs
mwbrooks Nov 26, 2025
6c020f7
Update cmd/collaborators/add.go
vegeris Nov 27, 2025
8e68452
Update cmd/collaborators/update.go
vegeris Nov 27, 2025
b12ed2b
error out when flag not provided
vegeris Nov 27, 2025
fa544dd
update error message
vegeris Nov 27, 2025
ea994f2
Merge branch 'main' into evegeris-release-read-only-app-collabs
mwbrooks Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 9 additions & 55 deletions cmd/collaborators/add.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/opentracing/opentracing-go"
"github.com/slackapi/slack-cli/internal/cmdutil"
"github.com/slackapi/slack-cli/internal/experiment"
"github.com/slackapi/slack-cli/internal/iostreams"
"github.com/slackapi/slack-cli/internal/prompts"
"github.com/slackapi/slack-cli/internal/shared"
Expand Down Expand Up @@ -62,8 +61,7 @@ func NewAddCommand(clients *shared.ClientFactory) *cobra.Command {
return runAddCommandFunc(ctx, clients, cmd, args)
},
}
cmd.Flags().StringVarP(&addFlags.permissionType, "permission-type", "P", "", "collaborator permission type: reader, owner")
cmd.Flag("permission-type").Hidden = true
cmd.Flags().StringVarP(&addFlags.permissionType, "permission-type", "P", "", "collaborator permission type: [reader|owner]")
Comment thread
vegeris marked this conversation as resolved.
Outdated
return cmd
}

Expand All @@ -87,7 +85,7 @@ func runAddCommandFunc(ctx context.Context, clients *shared.ClientFactory, cmd *
}
err = clients.API().AddCollaborator(ctx, selection.Auth.Token, selection.App.AppID, slackUser)
if err != nil {
if clients.Config.WithExperimentOn(experiment.ReadOnlyAppCollaborators) && strings.Contains(err.Error(), "user_already_owner") {
if strings.Contains(err.Error(), "user_already_owner") {
cmd.Println()
cmd.Println(style.Sectionf(style.TextSection{
Emoji: "bulb",
Expand Down Expand Up @@ -123,14 +121,15 @@ func promptCollaboratorsAdd(
if err != nil {
return types.SlackUser{}, err
}

switch clients.Config.Flags.Lookup("permission-type").Changed {
case true:
slackUser.PermissionType, err = promptCollaboratorsAddPermissionFlags(ctx, clients, addFlags.permissionType)
if err != nil {
return types.SlackUser{}, err
}
default:
slackUser.PermissionType, err = promptCollaboratorsAddPermissionPrompts(ctx, clients)
}
if err != nil {
return types.SlackUser{}, err
slackUser.PermissionType = types.OWNER
}
return slackUser, nil
}
Expand Down Expand Up @@ -181,40 +180,7 @@ func promptCollaboratorsAddSlackUserPrompts(
return slackUser, nil
}

// promptCollaboratorsAddPermissionPrompts gathers the collaborator permission
// from selection if the experiment allows
func promptCollaboratorsAddPermissionPrompts(

@vegeris vegeris Nov 20, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed the behaviour here: before when adding a new collaborator we required the permission type to be set, but I think we can default to 'owner' unless otherwise specified

@mwbrooks mwbrooks Nov 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds good!

Since we're now defaulting to the owner, we should update the description of the --permission-type to default to owner as well. I'll make a suggestion for that!

@vegeris vegeris Nov 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

ctx context.Context,
clients *shared.ClientFactory,
) (
permission types.AppCollaboratorPermission,
err error,
) {
switch clients.Config.WithExperimentOn(experiment.ReadOnlyAppCollaborators) {
case false:
return types.OWNER, nil
default:
permissionLabels := []string{
"owner",
"reader",
}
response, err := clients.IO.SelectPrompt(
ctx,
"Decide the collaborator permission",
permissionLabels,
iostreams.SelectPromptConfig{
Required: true,
},
)
if err != nil {
return "", err
}
return types.StringToAppCollaboratorPermission(response.Option)
}
}

// promptCollaboratorsAddPermissionFlags gathers the collaborator permission
// from flags if the experiment allows
// promptCollaboratorsAddPermissionFlags fetches collaborator permission from the flag
func promptCollaboratorsAddPermissionFlags(
ctx context.Context,
clients *shared.ClientFactory,
Expand All @@ -223,19 +189,7 @@ func promptCollaboratorsAddPermissionFlags(
permission types.AppCollaboratorPermission,
err error,
) {
switch clients.Config.WithExperimentOn(experiment.ReadOnlyAppCollaborators) {
case true:
return types.StringToAppCollaboratorPermission(addFlags.permissionType)
default:
clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{
Emoji: "construction",
Text: fmt.Sprintf("This command is under construction. Use at your own risk %s", style.Emoji("skull")),
Secondary: []string{
fmt.Sprintf("Bypass this message with the %s flag", style.Highlight("--experiment read-only-collaborators")),
},
}))
return "", slackerror.New(slackerror.ErrMissingExperiment)
}
return types.StringToAppCollaboratorPermission(addFlags.permissionType)
}

// printCollaboratorsAddSuccess outputs a message when addition is done
Expand Down
6 changes: 0 additions & 6 deletions cmd/collaborators/add_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ func TestAddCommand(t *testing.T) {
appSelectMock := prompts.NewAppSelectMock()
appSelectPromptFunc = appSelectMock.AppSelectPrompt
appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowHostedOnly, prompts.ShowInstalledAndUninstalledApps).Return(prompts.SelectedApp{App: types.App{AppID: "A123"}, Auth: types.SlackAuth{}}, nil)
// Set experiment flag
cm.Config.ExperimentsFlag = append(cm.Config.ExperimentsFlag, "read-only-collaborators")
cm.Config.LoadExperiments(ctx, cm.IO.PrintDebug)
// Mock API call
cm.API.On("AddCollaborator", mock.Anything, mock.Anything,
"A123",
Expand All @@ -61,9 +58,6 @@ func TestAddCommand(t *testing.T) {
appSelectMock := prompts.NewAppSelectMock()
appSelectPromptFunc = appSelectMock.AppSelectPrompt
appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowHostedOnly, prompts.ShowInstalledAndUninstalledApps).Return(prompts.SelectedApp{App: types.App{AppID: "A123"}, Auth: types.SlackAuth{}}, nil)
// Set experiment flag
cm.Config.ExperimentsFlag = append(cm.Config.ExperimentsFlag, "read-only-collaborators")
cm.Config.LoadExperiments(ctx, cm.IO.PrintDebug)
// Mock API call
cm.API.On("AddCollaborator", mock.Anything, mock.Anything,
"A123",
Expand Down
50 changes: 33 additions & 17 deletions cmd/collaborators/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
package collaborators

import (
"fmt"
"context"
"net/mail"

"github.com/opentracing/opentracing-go"
"github.com/slackapi/slack-cli/internal/cmdutil"
"github.com/slackapi/slack-cli/internal/experiment"
"github.com/slackapi/slack-cli/internal/iostreams"
"github.com/slackapi/slack-cli/internal/prompts"
"github.com/slackapi/slack-cli/internal/shared"
"github.com/slackapi/slack-cli/internal/shared/types"
Expand Down Expand Up @@ -51,23 +51,11 @@ func NewUpdateCommand(clients *shared.ClientFactory) *cobra.Command {
return cmdutil.IsValidProjectDirectory(clients)
},
RunE: func(cmd *cobra.Command, args []string) error {
if !clients.Config.WithExperimentOn(experiment.ReadOnlyAppCollaborators) {
cmd.Println()
cmd.Println(style.Sectionf(style.TextSection{
Emoji: "construction",
Text: fmt.Sprintf("This command is under construction. Use at your own risk %s", style.Emoji("skull")),
Secondary: []string{
fmt.Sprintf("Bypass this message with the %s flag", style.Highlight("--experiment read-only-collaborators")),
},
}))
return nil
}

return runUpdateCommand(cmd, clients, args)
},
}

cmd.Flags().StringVarP(&updateFlags.permissionType, "permission-type", "P", "", "collaborator permission type: reader, owner")
cmd.Flags().StringVarP(&updateFlags.permissionType, "permission-type", "P", "", "collaborator permission type: [reader|owner]")
Comment thread
vegeris marked this conversation as resolved.
Outdated

return cmd
}
Expand All @@ -92,8 +80,10 @@ func runUpdateCommand(cmd *cobra.Command, clients *shared.ClientFactory, args []
return err
}
} else {
cmd.Println(fmt.Sprintf("\n%s Specify a permission type for your collaborator with the %s flag\n", style.Emoji("warning"), style.Highlight("--permission-type")))
return nil
slackUser.PermissionType, err = promptCollaboratorPermissionSelection(ctx, clients)
if err != nil {
return err
}

@vegeris vegeris Nov 20, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When updating an existing collaborator however, maybe it would make sense to prompt for the permission type rather than error out

@mwbrooks mwbrooks Nov 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we choose to prompt, than we should handle non-TTY (no interactivity) as well. In that case, I think we would error when there is no flag value provided.

suggestion: Personally, I suggest we keep the error when the flag is not provided because it's been tested and works. If we switch to a prompt, I'd rather see it as a separate PR because it changes the functionality rather than simply unhides the experiment (purpose of this PR).

@vegeris vegeris Nov 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good; I'll flip this back to erroring out if the flag isn't provided. I'm fine with keeping that behaviour if the CLI team doesn't see adding the prompt as an enhancement to the user experience

}

// Get the app auth selection from the flag or prompt
Expand All @@ -117,3 +107,29 @@ func runUpdateCommand(cmd *cobra.Command, clients *shared.ClientFactory, args []

return nil
}

// promptCollaboratorPermissionSelection fetches collaborator permission from the prompt
func promptCollaboratorPermissionSelection(
ctx context.Context,
clients *shared.ClientFactory,
) (
permission types.AppCollaboratorPermission,
err error,
) {
permissionLabels := []string{
"owner",
"reader",
}
response, err := clients.IO.SelectPrompt(
ctx,
"Select a permission type",
permissionLabels,
iostreams.SelectPromptConfig{
Required: true,
},
)

@mwbrooks mwbrooks Nov 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion: I think this needs to be more robust. The iostreams.SelectPromptConfig should be bound to the flag and the flag, so that non-interactive environments can work properly with the prompt.

@vegeris vegeris Nov 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed the prompt for now but will make note of this if we want to bring it back

if err != nil {
return "", err
}
return types.StringToAppCollaboratorPermission(response.Option)
}
34 changes: 19 additions & 15 deletions cmd/collaborators/update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"testing"

"github.com/slackapi/slack-cli/internal/iostreams"
"github.com/slackapi/slack-cli/internal/prompts"
"github.com/slackapi/slack-cli/internal/shared"
"github.com/slackapi/slack-cli/internal/shared/types"
Expand All @@ -37,9 +38,6 @@ func TestUpdateCommand(t *testing.T) {
appSelectMock := prompts.NewAppSelectMock()
appSelectPromptFunc = appSelectMock.AppSelectPrompt
appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowHostedOnly, prompts.ShowInstalledAndUninstalledApps).Return(prompts.SelectedApp{App: types.App{AppID: "A123"}, Auth: types.SlackAuth{}}, nil)
// Set experiment flag
clientsMock.Config.ExperimentsFlag = append(clientsMock.Config.ExperimentsFlag, "read-only-collaborators")
clientsMock.Config.LoadExperiments(ctx, clientsMock.IO.PrintDebug)
// Mock APi call
clientsMock.API.On("UpdateCollaborator", mock.Anything, mock.Anything,
"A123",
Expand All @@ -55,33 +53,39 @@ func TestUpdateCommand(t *testing.T) {
appSelectMock := prompts.NewAppSelectMock()
appSelectPromptFunc = appSelectMock.AppSelectPrompt
appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowHostedOnly, prompts.ShowInstalledAndUninstalledApps).Return(prompts.SelectedApp{App: types.App{AppID: "A123"}, Auth: types.SlackAuth{}}, nil)
// Set experiment flag
clientsMock.Config.ExperimentsFlag = append(clientsMock.Config.ExperimentsFlag, "read-only-collaborators")
clientsMock.Config.LoadExperiments(ctx, clientsMock.IO.PrintDebug)
// Mock APi call
// Mock API call
clientsMock.API.On("UpdateCollaborator", mock.Anything, mock.Anything,
"A123",
types.SlackUser{Email: "joe.smith@company.com", PermissionType: types.OWNER}).Return(nil)
},
},
"permission type must be specified": {
"prompts when permission type not specified": {
CmdArgs: []string{"joe.smith@company.com"},
ExpectedOutputs: []string{"Specify a permission type for your collaborator"},
ExpectedOutputs: []string{"joe.smith@company.com successfully updated as a reader collaborator on this app"},
Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) {
clientsMock.AddDefaultMocks()
// Set experiment flag
clientsMock.Config.ExperimentsFlag = append(clientsMock.Config.ExperimentsFlag, "read-only-collaborators")
clientsMock.Config.LoadExperiments(ctx, clientsMock.IO.PrintDebug)
// Mock app selection
appSelectMock := prompts.NewAppSelectMock()
appSelectPromptFunc = appSelectMock.AppSelectPrompt
appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowHostedOnly, prompts.ShowInstalledAndUninstalledApps).Return(prompts.SelectedApp{App: types.App{AppID: "A123"}, Auth: types.SlackAuth{}}, nil)
// Mock permission selection prompt
clientsMock.IO.On("SelectPrompt", mock.Anything, "Select a permission type", mock.Anything, mock.Anything).Return(
iostreams.SelectPromptResponse{
Prompt: true,
Option: "reader",
Index: 1,
}, nil)
// Mock API call
clientsMock.API.On("UpdateCollaborator", mock.Anything, mock.Anything,
"A123",
types.SlackUser{Email: "joe.smith@company.com", PermissionType: types.READER}).Return(nil)
},
},
"user ID must be provided": {
CmdArgs: []string{},
ExpectedErrorStrings: []string{"accepts 1 arg(s), received 0"},
Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) {
clientsMock.AddDefaultMocks()
// Set experiment flag
clientsMock.Config.ExperimentsFlag = append(clientsMock.Config.ExperimentsFlag, "read-only-collaborators")
clientsMock.Config.LoadExperiments(ctx, clientsMock.IO.PrintDebug)
},
},
}, func(clients *shared.ClientFactory) *cobra.Command {
Expand Down
1 change: 0 additions & 1 deletion docs/reference/experiments.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ The following is a list of currently available experiments. We'll remove experim
* `bolt-install`: enables creating, installing, and running Bolt projects that manage their app manifest on app settings (remote manifest).
* `slack create` and `slack init` now set manifest source to "app settings" (remote) for Bolt JS & Bolt Python projects ([PR#96](https://github.com/slackapi/slack-cli/pull/96)).
* `slack run` and `slack install` support creating and installing Bolt Framework apps that have the manifest source set to "app settings (remote)" ([PR#111](https://github.com/slackapi/slack-cli/pull/111), [PR#154](https://github.com/slackapi/slack-cli/pull/154)).
* `read-only-collaborators`: enables creating and modifying collaborator permissions via the `slack collaborator` commands.

## Experiments changelog

Expand Down
5 changes: 0 additions & 5 deletions internal/experiment/experiment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ const (
// manage their app manifest on app settings (remote manifest).
BoltInstall Experiment = "bolt-install"

// The ReadOnlyAppCollaborators experiment enables creating and modifying collaborator
// permissions via the `collaborator` commands.
ReadOnlyAppCollaborators Experiment = "read-only-collaborators"

// Placeholder experiment is a placeholder for testing and does nothing... or does it?
Placeholder Experiment = "placeholder"
)
Expand All @@ -51,7 +47,6 @@ const (
var AllExperiments = []Experiment{
BoltFrameworks,
BoltInstall,
ReadOnlyAppCollaborators,
Placeholder,
}

Expand Down
1 change: 0 additions & 1 deletion internal/experiment/experiment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ func Test_Includes(t *testing.T) {

// Test expected experiments
require.Equal(t, true, Includes(Experiment("bolt")))
require.Equal(t, true, Includes(Experiment("read-only-collaborators")))

// Test invalid experiment
require.Equal(t, false, Includes(Experiment("should-fail")))
Expand Down
Loading