Skip to content
Merged
68 changes: 13 additions & 55 deletions cmd/collaborators/add.go
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,11 @@ 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", string(types.OWNER), fmt.Sprintf(
"collaborator permission type\n(\"%s\" or \"%s\")",
string(types.OWNER),
string(types.READER),
))
return cmd
}

Expand All @@ -87,7 +89,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 +125,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 +184,7 @@ func promptCollaboratorsAddSlackUserPrompts(
return slackUser, nil
}

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

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

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!

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 +193,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
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
19 changes: 5 additions & 14 deletions cmd/collaborators/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,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/prompts"
"github.com/slackapi/slack-cli/internal/shared"
"github.com/slackapi/slack-cli/internal/shared/types"
Expand Down Expand Up @@ -51,23 +50,15 @@ 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", "", fmt.Sprintf(
"collaborator permission type\n(\"%s\" or \"%s\")",
string(types.OWNER),
string(types.READER),
))

return cmd
}
Expand Down
11 changes: 1 addition & 10 deletions cmd/collaborators/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,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,10 +52,7 @@ 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)
Expand All @@ -79,9 +73,6 @@ func TestUpdateCommand(t *testing.T) {
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
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
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
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
2 changes: 1 addition & 1 deletion internal/shared/types/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func StringToAppCollaboratorPermission(input string) (AppCollaboratorPermission,
case "reader":
return READER, nil
default:
return "", fmt.Errorf("invalid")

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.

Updated this error message to be more specific

return "", fmt.Errorf("invalid permission; accepted values are \"owner\" or \"reader\"")
}
}

Expand Down
Loading