feat: update 'app link' to not change the app manifest source (#396)

* feat: default manifest source to local for non-Deno apps

Change the default manifest source for non-Deno (Bolt) projects from
ManifestSourceRemote to ManifestSourceLocal, aligning their behavior
with Deno projects.

* feat: update remote manifest warnings for link command and manifest overwrite prompt

* test: update tests for local and remote manifest source

* feat: fix preserving app manifest source when template sets it

* chore: remove redundant default for manifest source

* Update cmd/app/link.go

Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>

---------

Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>

Author: mwbrooks

cmd/app/link.go

@@ -16,7 +16,6 @@ package app
 
 import (
 	"context"
-	"fmt"
 	"path/filepath"
 	"strings"
 
@@ -36,9 +35,6 @@ import (
 // LinkAppConfirmPromptText is displayed when prompting to add an existing app
 const LinkAppConfirmPromptText = "Do you want to add an existing app?"
 
-// LinkAppManifestSourceConfirmPromptText is displayed before updating the manifest source
-const LinkAppManifestSourceConfirmPromptText = "Do you want to update the manifest source to remote?"
-
 // appLinkFlagSet contains flag values to reference
 type appLinkFlagSet struct {
 	environmentFlag string
@@ -131,11 +127,9 @@ func LinkAppHeaderSection(ctx context.Context, clients *shared.ClientFactory, sh
 }
 
 // LinkExistingApp prompts for an existing App ID and saves the details to the project.
-// When shouldConfirm is true, a confirmation prompt will ask the user is they want to
+// When shouldConfirm is true, a confirmation prompt will ask the user if they want to
 // link an existing app and additional information is included in the header.
 // The shouldConfirm option is encouraged for third-party callers.
-// The link command requires manifest source to be remote. When it is not, a
-// confirmation prompt is displayed before updating the manifest source value.
 func LinkExistingApp(ctx context.Context, clients *shared.ClientFactory, app *types.App, shouldConfirm bool) (err error) {
 	// Header section
 	LinkAppHeaderSection(ctx, clients, shouldConfirm)
@@ -156,68 +150,22 @@ func LinkExistingApp(ctx context.Context, clients *shared.ClientFactory, app *ty
 		}
 	}
 
-	// Confirm to update manifest source to remote.
-	// - Update the manifest source to remote when its a GBP project with a local manifest.
-	// - Do not update manifest source for ROSI projects, because they can only be local manifests.
+	// App Manifest section
 	manifestSource, err := clients.Config.ProjectConfig.GetManifestSource(ctx)
-	isManifestSourceRemote := manifestSource.Equals(config.ManifestSourceRemote)
-	isSlackHostedProject := cmdutil.IsSlackHostedProject(ctx, clients) == nil
-
-	if err != nil || (!isManifestSourceRemote && !isSlackHostedProject) {
-		// When undefined, the default is config.ManifestSourceLocal
-		if !manifestSource.Exists() {
-			manifestSource = config.ManifestSourceLocal
-		}
-
-		clients.IO.PrintInfo(ctx, false, "%s", style.Sectionf(style.TextSection{
-			Emoji: "warning",
-			Text:  "Warning",
-			Secondary: []string{
-				"Linking an existing app requires the app manifest source to be managed by",
-				fmt.Sprintf("%s.", config.ManifestSourceRemote.Human()),
-				" ",
-				fmt.Sprintf(`App manifest source can be %s or %s:`, config.ManifestSourceLocal.Human(), config.ManifestSourceRemote.Human()),
-				fmt.Sprintf("- %s: uses manifest from your project's source code for all apps", config.ManifestSourceLocal.String()),
-				fmt.Sprintf("- %s: uses manifest from app settings for each app", config.ManifestSourceRemote.String()),
-				" ",
-				fmt.Sprintf(style.Highlight(`Your manifest source is set to %s.`), manifestSource.Human()),
-				" ",
-				fmt.Sprintf("Current manifest source in %s:", style.Highlight(filepath.Join(config.ProjectConfigDirName, config.ProjectConfigJSONFilename))),
-				fmt.Sprintf(style.Highlight(`  %s: "%s"`), "manifest.source", manifestSource.String()),
-				" ",
-				fmt.Sprintf("Updating manifest source will be changed in %s:", style.Highlight(filepath.Join(config.ProjectConfigDirName, config.ProjectConfigJSONFilename))),
-				fmt.Sprintf(style.Highlight(`  %s: "%s"`), "manifest.source", config.ManifestSourceRemote),
-			},
-		}))
-
-		proceed, err := clients.IO.ConfirmPrompt(ctx, LinkAppManifestSourceConfirmPromptText, false)
-		if err != nil {
-			clients.IO.PrintDebug(ctx, "Error prompting to update the manifest source to %s: %s", config.ManifestSourceRemote, err)
-			return err
-		}
-
-		if !proceed {
-			// Add newline to match the trailing newline inserted from the footer section
-			clients.IO.PrintInfo(ctx, false, "")
-			return nil
-		}
-
-		if err := config.SetManifestSource(ctx, clients.Fs, clients.Os, config.ManifestSourceRemote); err != nil {
-			// Log the error to the verbose output
-			clients.IO.PrintDebug(ctx, "Error setting manifest source in project-level config: %s", err)
-			// Display a user-friendly error with a workaround
-			slackErr := slackerror.New(slackerror.ErrProjectConfigManifestSource).
-				WithMessage("Failed to update the manifest source to %s", config.ManifestSourceRemote).
-				WithRemediation(
-					"You can manually update the manifest source by setting the following\nproperty in %s:\n  %s",
-					filepath.Join(config.ProjectConfigDirName, config.ProjectConfigJSONFilename),
-					fmt.Sprintf(`manifest.source: "%s"`, config.ManifestSourceRemote),
-				).
-				WithRootCause(err)
-			clients.IO.PrintError(ctx, "%s", slackErr.Error())
-		}
+	if err != nil {
+		return err
 	}
 
+	configPath := filepath.Join(config.ProjectConfigDirName, config.ProjectConfigJSONFilename)
+	clients.IO.PrintInfo(ctx, false, "%s", style.Sectionf(style.TextSection{
+		Emoji: "books",
+		Text:  "App Manifest",
+		Secondary: []string{
+			"Manifest source is gathered from " + style.Highlight(manifestSource.Human()),
+			"Manifest source is configured in " + style.Highlight(configPath),
+		},
+	}))
+
 	// Prompt to get app details
 	var auth *types.SlackAuth
 	*app, auth, err = promptExistingApp(ctx, clients)

cmd/app/link_test.go

@@ -431,135 +431,18 @@ func Test_Apps_Link(t *testing.T) {
 			CmdArgs:       []string{},
 			ExpectedError: slackerror.New(slackerror.ErrAppNotFound),
 		},
-		"accepting manifest source prompt should save information about the provided deployed app": {
+		"links app when manifest source is local": {
 			Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) {
 				cm.Auth.On("Auths", mock.Anything).Return([]types.SlackAuth{
 					mockLinkSlackAuth2,
 					mockLinkSlackAuth1,
 				}, nil)
 				cm.AddDefaultMocks()
 				setupAppLinkCommandMocks(t, ctx, cm, cf)
-				// Set manifest source to project to trigger confirmation prompt
-				if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
-					require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
-				}
-				// Accept manifest source confirmation prompt
-				cm.IO.On("ConfirmPrompt",
-					mock.Anything,
-					LinkAppManifestSourceConfirmPromptText,
-					mock.Anything,
-				).Return(true, nil)
-				cm.IO.On("SelectPrompt",
-					mock.Anything,
-					"Select the existing app team",
-					mock.Anything,
-					mock.Anything,
-					mock.Anything,
-				).Return(iostreams.SelectPromptResponse{
-					Flag:   true,
-					Option: mockLinkSlackAuth1.TeamDomain,
-				}, nil)
-				cm.IO.On("InputPrompt",
-					mock.Anything,
-					"Enter the existing app ID",
-					mock.Anything,
-				).Return(mockLinkAppID1, nil)
-				cm.IO.On("SelectPrompt",
-					mock.Anything,
-					"Choose the app environment",
-					mock.Anything,
-					mock.Anything,
-					mock.Anything,
-				).Return(iostreams.SelectPromptResponse{
-					Flag:   true,
-					Option: "deployed",
-				}, nil)
-				cm.API.On(
-					"GetAppStatus",
-					mock.Anything,
-					mockLinkSlackAuth1.Token,
-					[]string{mockLinkAppID1},
-					mockLinkSlackAuth1.TeamID,
-				).Return(api.GetAppStatusResult{}, nil)
-			},
-			CmdArgs: []string{},
-			ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {
-				expectedApp := types.App{
-					AppID:        mockLinkAppID1,
-					TeamDomain:   mockLinkSlackAuth1.TeamDomain,
-					TeamID:       mockLinkSlackAuth1.TeamID,
-					EnterpriseID: mockLinkSlackAuth1.EnterpriseID,
-				}
-				actualApp, err := cm.AppClient.GetDeployed(
-					ctx,
-					mockLinkSlackAuth1.TeamID,
-				)
-				require.NoError(t, err)
-				assert.Equal(t, expectedApp, actualApp)
-				// Assert manifest confirmation prompt accepted
-				cm.IO.AssertCalled(t, "ConfirmPrompt",
-					mock.Anything,
-					LinkAppManifestSourceConfirmPromptText,
-					mock.Anything,
-				)
-			},
-		},
-		"declining manifest source prompt should not link app": {
-			Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) {
-				cm.AddDefaultMocks()
-				setupAppLinkCommandMocks(t, ctx, cm, cf)
-				// Set manifest source to project to trigger confirmation prompt
-				if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
-					require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
-				}
-				// Decline manifest source confirmation prompt
-				cm.IO.On("ConfirmPrompt",
-					mock.Anything,
-					LinkAppManifestSourceConfirmPromptText,
-					mock.Anything,
-				).Return(false, nil)
-			},
-			CmdArgs: []string{},
-			ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {
-				// Assert manifest confirmation prompt accepted
-				cm.IO.AssertCalled(t, "ConfirmPrompt",
-					mock.Anything,
-					LinkAppManifestSourceConfirmPromptText,
-					mock.Anything,
-				)
-
-				// Assert no apps saved
-				apps, _, err := cm.AppClient.GetDeployedAll(ctx)
-				require.NoError(t, err)
-				require.Len(t, apps, 0)
-
-				apps, err = cm.AppClient.GetLocalAll(ctx)
-				require.NoError(t, err)
-				require.Len(t, apps, 0)
-			},
-		},
-		"manifest source prompt should not display for Run-on-Slack apps with local manifest source": {
-			Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) {
-				cm.Auth.On("Auths", mock.Anything).Return([]types.SlackAuth{
-					mockLinkSlackAuth1,
-					mockLinkSlackAuth2,
-				}, nil)
-				cm.AddDefaultMocks()
-				setupAppLinkCommandMocks(t, ctx, cm, cf)
 				// Set manifest source to local
 				if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
 					require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
 				}
-				// Mock manifest for Run-on-Slack app
-				manifestMock := &app.ManifestMockObject{}
-				manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{
-					AppManifest: types.AppManifest{
-						Settings: &types.AppSettings{
-							FunctionRuntime: types.SlackHosted,
-						},
-					},
-				}, nil)
-				cf.AppClient().Manifest = manifestMock
 				cm.IO.On("SelectPrompt",
 					mock.Anything,
 					"Select the existing app team",
@@ -583,7 +466,7 @@ func Test_Apps_Link(t *testing.T) {
 					mock.Anything,
 				).Return(iostreams.SelectPromptResponse{
 					Flag:   true,
-					Option: "deployed",
+					Option: "local",
 				}, nil)
 				cm.API.On(
 					"GetAppStatus",
@@ -600,42 +483,33 @@ func Test_Apps_Link(t *testing.T) {
 					TeamDomain:   mockLinkSlackAuth1.TeamDomain,
 					TeamID:       mockLinkSlackAuth1.TeamID,
 					EnterpriseID: mockLinkSlackAuth1.EnterpriseID,
+					UserID:       mockLinkSlackAuth1.UserID,
+					IsDev:        true,
 				}
-				actualApp, err := cm.AppClient.GetDeployed(
+				actualApp, err := cm.AppClient.GetLocal(
 					ctx,
 					mockLinkSlackAuth1.TeamID,
 				)
 				require.NoError(t, err)
 				assert.Equal(t, expectedApp, actualApp)
-				// Assert manifest confirmation prompt was not displayed
-				cm.IO.AssertNotCalled(t, "ConfirmPrompt",
-					mock.Anything,
-					LinkAppManifestSourceConfirmPromptText,
-					mock.Anything,
-				)
+				// Assert manifest source info is displayed
+				output := cm.GetCombinedOutput()
+				assert.Contains(t, output, "App Manifest")
+				assert.Contains(t, output, `"project" (local)`)
 			},
 		},
-		"manifest source prompt should display for GBP apps with local manifest source": {
+		"links app when manifest source is remote": {
 			Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) {
 				cm.Auth.On("Auths", mock.Anything).Return([]types.SlackAuth{
-					mockLinkSlackAuth1,
 					mockLinkSlackAuth2,
+					mockLinkSlackAuth1,
 				}, nil)
 				cm.AddDefaultMocks()
 				setupAppLinkCommandMocks(t, ctx, cm, cf)
-				// Set manifest source to local
-				if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
+				// Set manifest source to remote
+				if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceRemote); err != nil {
 					require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
 				}
-				// Mock manifest for Run-on-Slack app
-				manifestMock := &app.ManifestMockObject{}
-				manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{}, nil)
-				cf.AppClient().Manifest = manifestMock
-				cm.IO.On("ConfirmPrompt",
-					mock.Anything,
-					LinkAppManifestSourceConfirmPromptText,
-					mock.Anything,
-				).Return(true, nil)
 				cm.IO.On("SelectPrompt",
 					mock.Anything,
 					"Select the existing app team",
@@ -659,7 +533,7 @@ func Test_Apps_Link(t *testing.T) {
 					mock.Anything,
 				).Return(iostreams.SelectPromptResponse{
 					Flag:   true,
-					Option: "deployed",
+					Option: "local",
 				}, nil)
 				cm.API.On(
 					"GetAppStatus",
@@ -676,19 +550,19 @@ func Test_Apps_Link(t *testing.T) {
 					TeamDomain:   mockLinkSlackAuth1.TeamDomain,
 					TeamID:       mockLinkSlackAuth1.TeamID,
 					EnterpriseID: mockLinkSlackAuth1.EnterpriseID,
+					UserID:       mockLinkSlackAuth1.UserID,
+					IsDev:        true,
 				}
-				actualApp, err := cm.AppClient.GetDeployed(
+				actualApp, err := cm.AppClient.GetLocal(
 					ctx,
 					mockLinkSlackAuth1.TeamID,
 				)
 				require.NoError(t, err)
 				assert.Equal(t, expectedApp, actualApp)
-				// Assert manifest confirmation prompt was displayed
-				cm.IO.AssertCalled(t, "ConfirmPrompt",
-					mock.Anything,
-					LinkAppManifestSourceConfirmPromptText,
-					mock.Anything,
-				)
+				// Assert manifest source info is displayed
+				output := cm.GetCombinedOutput()
+				assert.Contains(t, output, "App Manifest")
+				assert.Contains(t, output, `"app settings" (remote)`)
 			},
 		},
 	}, func(clients *shared.ClientFactory) *cobra.Command {

cmd/project/init_test.go

@@ -108,10 +108,8 @@ func Test_Project_InitCommand(t *testing.T) {
 				}, nil)
 				// Default setup
 				setupProjectInitCommandMocks(t, ctx, cm, cf)
-				// Do not link an existing app
+				// Link an existing app
 				cm.IO.On("ConfirmPrompt", mock.Anything, app.LinkAppConfirmPromptText, mock.Anything).Return(true, nil)
-				// Mock prompt to link an existing app
-				cm.IO.On("ConfirmPrompt", mock.Anything, app.LinkAppManifestSourceConfirmPromptText, mock.Anything).Return(true, nil)
 				// Mock prompt for team
 				cm.IO.On("SelectPrompt",
 					mock.Anything,

internal/pkg/apps/install.go

@@ -752,7 +752,7 @@ func shouldUpdateManifest(ctx context.Context, clients *shared.ClientFactory, ap
 	case saved.Equals(""):
 		notice = "Manifest values for this app are overwritten on reinstall"
 	default:
-		notice = "The manifest on app settings has been changed since last update!"
+		notice = style.Yellow("The manifest on app settings has been changed since last update")
 	}
 	clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{
 		Emoji:     "books",
@@ -764,10 +764,7 @@ func shouldUpdateManifest(ctx context.Context, clients *shared.ClientFactory, ap
 	}
 	continues, err := clients.IO.ConfirmPrompt(
 		ctx,
-		fmt.Sprintf(
-			"Update app settings with changes to the %s manifest?",
-			config.ManifestSourceLocal.String(),
-		),
+		"Overwrite manifest on app settings with the project's manifest file?",
 		false,
 	)
 	if err != nil {

internal/pkg/apps/install_test.go

@@ -574,7 +574,7 @@ func TestInstall(t *testing.T) {
 				clientsMock.IO.On(
 					"ConfirmPrompt",
 					mock.Anything,
-					"Update app settings with changes to the local manifest?",
+					"Overwrite manifest on app settings with the project's manifest file?",
 					false,
 				).Return(
 					tc.mockConfirmPrompt,
@@ -1396,7 +1396,7 @@ func TestInstallLocalApp(t *testing.T) {
 				clientsMock.IO.On(
 					"ConfirmPrompt",
 					mock.Anything,
-					"Update app settings with changes to the local manifest?",
+					"Overwrite manifest on app settings with the project's manifest file?",
 					false,
 				).Return(
 					tc.mockConfirmPrompt,