diff --git a/cmd/env/add.go b/cmd/env/add.go index 8fbb0944..8ca04424 100644 --- a/cmd/env/add.go +++ b/cmd/env/add.go @@ -85,44 +85,49 @@ func preRunEnvAddCommandFunc(ctx context.Context, clients *shared.ClientFactory, func runEnvAddCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, args []string) error { ctx := cmd.Context() - // Get the workspace from the flag or prompt - selection, err := appSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAppsOnly) - if err != nil { - return err + // Hosted apps require selecting an app before gathering variable inputs. + hosted := isHostedRuntime(ctx, clients) + var selection prompts.SelectedApp + if hosted { + s, err := appSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAppsOnly) + if err != nil { + return err + } + selection = s } // Get the variable name from the args or prompt - var variableName string + variableName := "" if len(args) < 1 { - variableName, err = clients.IO.InputPrompt(ctx, "Variable name", iostreams.InputPromptConfig{ + name, err := clients.IO.InputPrompt(ctx, "Variable name", iostreams.InputPromptConfig{ Required: false, }) if err != nil { return err } + variableName = name } else { variableName = args[0] } // Get the variable value from the args or prompt - var variableValue string + variableValue := "" if len(args) < 2 { response, err := clients.IO.PasswordPrompt(ctx, "Variable value", iostreams.PasswordPromptConfig{ Flag: clients.Config.Flags.Lookup("value"), }) if err != nil { return err - } else { - variableValue = response.Value } + variableValue = response.Value } else { variableValue = args[1] } // Add the environment variable using either the Slack API method or the // project ".env" file depending on the app hosting. - if !selection.App.IsDev && cmdutil.IsSlackHostedProject(ctx, clients) == nil { - err = clients.API().AddVariable( + if hosted && !selection.App.IsDev { + err := clients.API().AddVariable( ctx, selection.Auth.Token, selection.App.AppID, diff --git a/cmd/env/add_test.go b/cmd/env/add_test.go index 377dda9f..8b13ffb0 100644 --- a/cmd/env/add_test.go +++ b/cmd/env/add_test.go @@ -204,33 +204,16 @@ func Test_Env_AddCommand(t *testing.T) { ) }, }, - "add a numeric variable using prompts to the .env file": { - CmdArgs: []string{}, + "add a numeric variable to the .env file for remote runtime": { + CmdArgs: []string{"PORT", "3000"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { setupEnvAddDotenvMocks(ctx, cm, cf) - cm.IO.On( - "InputPrompt", - mock.Anything, - "Variable name", - mock.Anything, - ).Return( - "PORT", - nil, - ) - cm.IO.On( - "PasswordPrompt", - mock.Anything, - "Variable value", - iostreams.MatchPromptConfig(iostreams.PasswordPromptConfig{ - Flag: cm.Config.Flags.Lookup("value"), - }), - ).Return( - iostreams.PasswordPromptResponse{ - Prompt: true, - Value: "3000", - }, + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return( + types.SlackYaml{AppManifest: types.AppManifest{Settings: &types.AppSettings{FunctionRuntime: types.Remote}}}, nil, ) + cm.AppClient.Manifest = manifestMock }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertNotCalled(t, "AddVariable") @@ -239,10 +222,13 @@ func Test_Env_AddCommand(t *testing.T) { assert.Equal(t, "PORT=3000\n", string(content)) }, }, - "add a variable to the .env file for non-hosted app": { + "add a variable to the .env file when no runtime is set": { CmdArgs: []string{"NEW_VAR", "new_value"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { setupEnvAddDotenvMocks(ctx, cm, cf) + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{}, nil) + cm.AppClient.Manifest = manifestMock err := afero.WriteFile(cf.Fs, ".env", []byte("# Config\nEXISTING=value\n"), 0600) assert.NoError(t, err) }, @@ -250,7 +236,25 @@ func Test_Env_AddCommand(t *testing.T) { cm.API.AssertNotCalled(t, "AddVariable") content, err := afero.ReadFile(cm.Fs, ".env") assert.NoError(t, err) - assert.Equal(t, "# Config\nEXISTING=value\nNEW_VAR=\"new_value\"\n", string(content)) + assert.Equal(t, "# Config\nEXISTING=value\n"+`NEW_VAR="new_value"`+"\n", string(content)) + }, + }, + "add a variable to the .env file when manifest fetch errors": { + CmdArgs: []string{"API_KEY", "sk-1234"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + setupEnvAddDotenvMocks(ctx, cm, cf) + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return( + types.SlackYaml{}, + slackerror.New(slackerror.ErrSDKHookNotFound), + ) + cm.AppClient.Manifest = manifestMock + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "AddVariable") + content, err := afero.ReadFile(cm.Fs, ".env") + assert.NoError(t, err) + assert.Equal(t, `API_KEY="sk-1234"`+"\n", string(content)) }, }, }, func(cf *shared.ClientFactory) *cobra.Command { @@ -292,20 +296,11 @@ func setupEnvAddHostedMocks(ctx context.Context, cm *shared.ClientsMock, cf *sha cf.SDKConfig.WorkingDirectory = "/slack/path/to/project" } -// setupEnvAddDotenvMocks prepares common mocks for non-hosted (dotenv) app tests +// setupEnvAddDotenvMocks prepares common mocks for non-hosted (dotenv) app tests. +// Callers must set their own manifest mock on cm.AppClient.Manifest. func setupEnvAddDotenvMocks(_ context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { cf.SDKConfig = hooks.NewSDKConfigMock() cm.AddDefaultMocks() - mockDevApp := types.App{ - TeamID: "T1", - TeamDomain: "team1", - AppID: "A0123456789", - IsDev: true, - } - appSelectMock := prompts.NewAppSelectMock() - appSelectPromptFunc = appSelectMock.AppSelectPrompt - appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowAllEnvironments, prompts.ShowInstalledAppsOnly).Return(prompts.SelectedApp{Auth: mockAuth, App: mockDevApp}, nil) - cm.Config.Flags.String("value", "", "mock value flag") } diff --git a/cmd/env/env.go b/cmd/env/env.go index abf6e9ea..4a6a929c 100644 --- a/cmd/env/env.go +++ b/cmd/env/env.go @@ -15,6 +15,7 @@ package env import ( + "context" "strings" "github.com/slackapi/slack-cli/internal/prompts" @@ -72,3 +73,16 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command { return cmd } + +// isHostedRuntime returns true if the local manifest is for an app that uses +// the Deno Slack SDK function runtime. +// +// It defaults to false when the manifest cannot be fetched, which directs the +// command to use the project ".env" file. Otherwise the API is used. +func isHostedRuntime(ctx context.Context, clients *shared.ClientFactory) bool { + manifest, err := clients.AppClient().Manifest.GetManifestLocal(ctx, clients.SDKConfig, clients.HookExecutor) + if err != nil { + return false + } + return manifest.IsFunctionRuntimeSlackHosted() || manifest.IsFunctionRuntimeLocal() +} diff --git a/cmd/env/env_test.go b/cmd/env/env_test.go index ebca06b7..064d629e 100644 --- a/cmd/env/env_test.go +++ b/cmd/env/env_test.go @@ -17,9 +17,15 @@ package env import ( "testing" + "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" + "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/test/testutil" "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func Test_Env_Command(t *testing.T) { @@ -36,3 +42,65 @@ func Test_Env_Command(t *testing.T) { return cmd }) } + +func Test_isHostedRuntime(t *testing.T) { + tests := map[string]struct { + mockManifest types.SlackYaml + mockError error + expected bool + }{ + "returns true for slack hosted runtime": { + mockManifest: types.SlackYaml{ + AppManifest: types.AppManifest{ + Settings: &types.AppSettings{ + FunctionRuntime: types.SlackHosted, + }, + }, + }, + expected: true, + }, + "returns true for local runtime": { + mockManifest: types.SlackYaml{ + AppManifest: types.AppManifest{ + Settings: &types.AppSettings{ + FunctionRuntime: types.LocallyRun, + }, + }, + }, + expected: true, + }, + "returns false for remote runtime": { + mockManifest: types.SlackYaml{ + AppManifest: types.AppManifest{ + Settings: &types.AppSettings{ + FunctionRuntime: types.Remote, + }, + }, + }, + expected: false, + }, + "returns false for empty runtime": { + mockManifest: types.SlackYaml{ + AppManifest: types.AppManifest{ + Settings: &types.AppSettings{}, + }, + }, + expected: false, + }, + "returns false when manifest fetch fails": { + mockError: slackerror.New(slackerror.ErrSDKHookInvocationFailed), + expected: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + clientsMock := shared.NewClientsMock() + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(tc.mockManifest, tc.mockError) + clientsMock.AppClient.Manifest = manifestMock + clients := shared.NewClientFactory(clientsMock.MockClientFactory()) + assert.Equal(t, tc.expected, isHostedRuntime(ctx, clients)) + }) + } +} diff --git a/cmd/env/list.go b/cmd/env/list.go index eca3cb72..5f53c7fc 100644 --- a/cmd/env/list.go +++ b/cmd/env/list.go @@ -73,20 +73,27 @@ func runEnvListCommandFunc( ) error { ctx := cmd.Context() - selection, err := appSelectPromptFunc( - ctx, - clients, - prompts.ShowAllEnvironments, - prompts.ShowInstalledAppsOnly, - ) - if err != nil { - return err + // Hosted apps require selecting an app before gathering variables. + hosted := isHostedRuntime(ctx, clients) + var selection prompts.SelectedApp + if hosted { + var err error + selection, err = appSelectPromptFunc( + ctx, + clients, + prompts.ShowAllEnvironments, + prompts.ShowInstalledAppsOnly, + ) + if err != nil { + return err + } } - // Gather environment variables for either a ROSI app from the Slack API method + // Gather environment variables from the Slack API for deployed hosted apps // or read from project files. var variableNames []string - if !selection.App.IsDev && cmdutil.IsSlackHostedProject(ctx, clients) == nil { + if hosted && !selection.App.IsDev { + var err error variableNames, err = clients.API().ListVariables( ctx, selection.Auth.Token, diff --git a/cmd/env/list_test.go b/cmd/env/list_test.go index 5e2b054d..6d2a35ed 100644 --- a/cmd/env/list_test.go +++ b/cmd/env/list_test.go @@ -19,7 +19,6 @@ import ( "testing" "github.com/slackapi/slack-cli/internal/app" - "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" @@ -71,9 +70,14 @@ func Test_Env_ListCommand(t *testing.T) { } testutil.TableTestCommand(t, testutil.CommandTests{ - "lists variables from the .env file": { + "lists variables from the .env file for remote runtime": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - mockAppSelect() + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return( + types.SlackYaml{AppManifest: types.AppManifest{Settings: &types.AppSettings{FunctionRuntime: types.Remote}}}, + nil, + ) + cm.AppClient.Manifest = manifestMock err := afero.WriteFile(cf.Fs, ".env", []byte("SECRET_KEY=abc123\nAPI_TOKEN=xyz789\n"), 0644) assert.NoError(t, err) }, @@ -101,7 +105,9 @@ func Test_Env_ListCommand(t *testing.T) { }, "lists no variables when the .env file does not exist": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - mockAppSelect() + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{}, nil) + cm.AppClient.Manifest = manifestMock }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.IO.AssertCalled( @@ -117,7 +123,12 @@ func Test_Env_ListCommand(t *testing.T) { }, "lists no variables when the .env file is empty": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - mockAppSelect() + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return( + types.SlackYaml{}, + slackerror.New(slackerror.ErrSDKHookNotFound), + ) + cm.AppClient.Manifest = manifestMock err := afero.WriteFile(cf.Fs, ".env", []byte(""), 0644) assert.NoError(t, err) }, @@ -161,9 +172,6 @@ func Test_Env_ListCommand(t *testing.T) { nil, ) cm.AppClient.Manifest = manifestMock - projectConfigMock := config.NewProjectConfigMock() - projectConfigMock.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) - cm.Config.ProjectConfig = projectConfigMock cf.SDKConfig.WorkingDirectory = "/slack/path/to/project" }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { diff --git a/internal/shared/types/app_manifest.go b/internal/shared/types/app_manifest.go index 8d30c5b9..5c875b53 100644 --- a/internal/shared/types/app_manifest.go +++ b/internal/shared/types/app_manifest.go @@ -321,6 +321,12 @@ func (manifest *AppManifest) FunctionRuntime() FunctionRuntime { return manifest.Settings.FunctionRuntime } +// IsFunctionRuntimeLocal returns true when the function runtime setting +// is local +func (manifest *AppManifest) IsFunctionRuntimeLocal() bool { + return manifest.Settings != nil && manifest.Settings.FunctionRuntime == LocallyRun +} + // IsFunctionRuntimeSlackHosted returns true when the function runtime setting // is slack hosted func (manifest *AppManifest) IsFunctionRuntimeSlackHosted() bool { diff --git a/internal/shared/types/app_manifest_test.go b/internal/shared/types/app_manifest_test.go index 02a64354..0d57b23b 100644 --- a/internal/shared/types/app_manifest_test.go +++ b/internal/shared/types/app_manifest_test.go @@ -305,41 +305,49 @@ func Test_AppManifest_AppSettings_FunctionRuntime(t *testing.T) { tests := map[string]struct { settings *AppSettings expectedHosted bool + expectedLocal bool expectedRuntime FunctionRuntime }{ "undefined settings have no function runtime": { settings: nil, expectedHosted: false, + expectedLocal: false, expectedRuntime: "", }, "undefined function runtime has no function runtime": { settings: &AppSettings{}, expectedHosted: false, + expectedLocal: false, expectedRuntime: "", }, "setting the function runtime to slack is hosted": { settings: &AppSettings{FunctionRuntime: "slack"}, expectedHosted: true, + expectedLocal: false, expectedRuntime: SlackHosted, }, "setting the function runtime to remote is not hosted": { settings: &AppSettings{FunctionRuntime: "remote"}, expectedHosted: false, + expectedLocal: false, expectedRuntime: Remote, }, - "setting the function runtime to local is not hosted": { + "setting the function runtime to local is local": { settings: &AppSettings{FunctionRuntime: "local"}, expectedHosted: false, + expectedLocal: true, expectedRuntime: LocallyRun, }, "setting the function runtime to random is not hosted": { settings: &AppSettings{FunctionRuntime: "sparkling-butterflies"}, expectedHosted: false, + expectedLocal: false, expectedRuntime: "sparkling-butterflies", }, "setting the function runtime to padded string is possible": { settings: &AppSettings{FunctionRuntime: " "}, expectedHosted: false, + expectedLocal: false, expectedRuntime: " ", }, } @@ -349,6 +357,7 @@ func Test_AppManifest_AppSettings_FunctionRuntime(t *testing.T) { Settings: tc.settings, } assert.Equal(t, tc.expectedHosted, manifest.IsFunctionRuntimeSlackHosted()) + assert.Equal(t, tc.expectedLocal, manifest.IsFunctionRuntimeLocal()) assert.Equal(t, tc.expectedRuntime, manifest.FunctionRuntime()) if tc.settings != nil { assert.Equal(t, tc.expectedRuntime, manifest.Settings.FunctionRuntime)