diff --git a/docs/reference/experiments.md b/docs/reference/experiments.md index e2c57ca5..b974e23a 100644 --- a/docs/reference/experiments.md +++ b/docs/reference/experiments.md @@ -8,10 +8,12 @@ The following is a list of currently available experiments. We'll remove experim - `lipgloss`: shows pretty styles. - `sandboxes`: enables users who have joined the Slack Developer Program to manage their sandboxes ([PR#379](https://github.com/slackapi/slack-cli/pull/379)). +- `set-icon`: enables icon upload for non-hosted apps ([PR#469](https://github.com/slackapi/slack-cli/pull/469)). ## Experiments changelog Below is a list of updates related to experiments. +- **April 2026**: Added the `set-icon` experiment to enable icon upload for non-hosted apps. - **April 2026**: Concluded the `huh` experiment with full support now enabled by default in the Slack CLI. - **March 2026**: Split the `charm` experiment into more beautiful `huh` prompts and prettier `lipgloss` styles for ongoing change. - **March 2026**: Concluded the `bolt` and `bolt-install` experiments with full Bolt framework support now enabled by default in the Slack CLI. All Bolt project features including remote manifest management are now standard functionality. See the announcement [here](https://slack.dev/slackcli-supports-bolt-apps/). diff --git a/internal/api/app.go b/internal/api/app.go index 32bb1df0..a871dcd5 100644 --- a/internal/api/app.go +++ b/internal/api/app.go @@ -59,6 +59,7 @@ type AppsClient interface { GetPresignedS3PostParams(ctx context.Context, token string, appID string) (GenerateS3PresignedPostResult, error) Host() string Icon(ctx context.Context, fs afero.Fs, token, appID, iconFilePath string) (IconResult, error) + IconSet(ctx context.Context, fs afero.Fs, token, appID, iconFilePath string) (IconResult, error) RequestAppApproval(ctx context.Context, token string, appID string, teamID string, reason string, scopes string, outgoingDomains []string) (AppsApprovalsRequestsCreateResult, error) SetHost(host string) UninstallApp(ctx context.Context, token string, appID, teamID string) error diff --git a/internal/api/icon.go b/internal/api/icon.go index efee1f4f..5be65442 100644 --- a/internal/api/icon.go +++ b/internal/api/icon.go @@ -35,6 +35,8 @@ import ( const ( appIconMethod = "apps.hosted.icon" + // appIconSetMethod is the API method for setting app icons for non-hosted apps. + appIconSetMethod = "apps.icon.set" ) // IconResult details to be saved @@ -46,8 +48,19 @@ type iconResponse struct { IconResult } -// Icon updates a Slack App's icon +// Icon updates a hosted Slack app icon +// DEPRECATED: Prefer "IconSet" instead func (c *Client) Icon(ctx context.Context, fs afero.Fs, token, appID, iconFilePath string) (IconResult, error) { + return c.uploadIcon(ctx, fs, token, appID, iconFilePath, appIconMethod, "file") +} + +// IconSet sets a Slack App's icon using the apps.icon.set API method. +func (c *Client) IconSet(ctx context.Context, fs afero.Fs, token, appID, iconFilePath string) (IconResult, error) { + return c.uploadIcon(ctx, fs, token, appID, iconFilePath, appIconSetMethod, "file") +} + +// uploadIcon uploads an icon to the given API method. +func (c *Client) uploadIcon(ctx context.Context, fs afero.Fs, token, appID, iconFilePath, apiMethod, fileFieldName string) (IconResult, error) { var ( iconBytes []byte err error @@ -81,7 +94,7 @@ func (c *Client) Icon(ctx context.Context, fs afero.Fs, token, appID, iconFilePa var part io.Writer h := make(textproto.MIMEHeader) - h.Set("Content-Disposition", fmt.Sprintf(`form-data; name="%s"; filename="%s"`, "file", iconStat.Name())) + h.Set("Content-Disposition", fmt.Sprintf(`form-data; name="%s"; filename="%s"`, fileFieldName, iconStat.Name())) h.Set("Content-Type", http.DetectContentType(iconBytes)) part, err = writer.CreatePart(h) if err != nil { @@ -101,7 +114,7 @@ func (c *Client) Icon(ctx context.Context, fs afero.Fs, token, appID, iconFilePa writer.Close() var sURL *url.URL - sURL, err = url.Parse(c.host + "/api/" + appIconMethod) + sURL, err = url.Parse(c.host + "/api/" + apiMethod) if err != nil { return IconResult{}, err } diff --git a/internal/api/icon_mock.go b/internal/api/icon_mock.go index f8b0a998..4e792083 100644 --- a/internal/api/icon_mock.go +++ b/internal/api/icon_mock.go @@ -21,6 +21,11 @@ import ( ) func (m *APIMock) Icon(ctx context.Context, fs afero.Fs, token, appID, iconFilePath string) (IconResult, error) { - args := m.Called(ctx, fs, token, iconFilePath) + args := m.Called(ctx, fs, token, appID, iconFilePath) + return args.Get(0).(IconResult), args.Error(1) +} + +func (m *APIMock) IconSet(ctx context.Context, fs afero.Fs, token, appID, iconFilePath string) (IconResult, error) { + args := m.Called(ctx, fs, token, appID, iconFilePath) return args.Get(0).(IconResult), args.Error(1) } diff --git a/internal/api/icon_test.go b/internal/api/icon_test.go index 1197e67d..d254209c 100644 --- a/internal/api/icon_test.go +++ b/internal/api/icon_test.go @@ -28,65 +28,221 @@ import ( var imgFile = ".assets/icon.png" -func TestClient_IconErrorIfMissingArgs(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - fs := afero.NewMemMapFs() - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: appIconMethod, - }) - defer teardown() - _, err := c.Icon(ctx, fs, "token", "", "") - require.Error(t, err) - require.Contains(t, err.Error(), "missing required args") -} - -func TestClient_IconErrorNoFile(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - fs := afero.NewMemMapFs() - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: appIconMethod, - }) - defer teardown() - _, err := c.Icon(ctx, fs, "token", "12345", imgFile) - require.Error(t, err) - require.Contains(t, err.Error(), "file does not exist") -} - -func TestClient_IconErrorWrongFile(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - fs := afero.NewMemMapFs() - err := afero.WriteFile(fs, "test.txt", []byte("this is a text file"), 0666) - require.NoError(t, err) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: appIconMethod, - }) - defer teardown() - _, err = c.Icon(ctx, fs, "token", "12345", "test.txt") - require.Error(t, err) - require.Contains(t, err.Error(), "unknown format") -} - -func TestClient_IconSuccess(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - fs := afero.NewMemMapFs() - +func createTestPNG(t *testing.T, fs afero.Fs, path string) { + t.Helper() myimage := image.NewRGBA(image.Rectangle{image.Point{0, 0}, image.Point{100, 100}}) - - // This loop just fills the image with random data for x := range 100 { for y := range 100 { c := color.RGBA{uint8(rand.Intn(255)), uint8(rand.Intn(255)), uint8(rand.Intn(255)), 255} myimage.Set(x, y, c) } } - myfile, _ := fs.Create(imgFile) - err := png.Encode(myfile, myimage) + myfile, err := fs.Create(path) require.NoError(t, err) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: appIconMethod, - Response: `{"ok":true}`, - }) - defer teardown() - _, err = c.Icon(ctx, fs, "token", "12345", imgFile) + err = png.Encode(myfile, myimage) require.NoError(t, err) } + +func TestClient_Icon(t *testing.T) { + tests := map[string]struct { + setupFs func(t *testing.T, fs afero.Fs) + appID string + filePath string + response string + expectErr bool + errContains string + }{ + "returns error when args are missing": { + appID: "", + filePath: "", + expectErr: true, + errContains: "missing required args", + }, + "returns error when file does not exist": { + appID: "12345", + filePath: imgFile, + expectErr: true, + errContains: "file does not exist", + }, + "returns error for non-image file": { + setupFs: func(t *testing.T, fs afero.Fs) { + err := afero.WriteFile(fs, "test.txt", []byte("this is a text file"), 0666) + require.NoError(t, err) + }, + appID: "12345", + filePath: "test.txt", + expectErr: true, + errContains: "unknown format", + }, + "succeeds with valid PNG": { + setupFs: func(t *testing.T, fs afero.Fs) { + createTestPNG(t, fs, imgFile) + }, + appID: "12345", + filePath: imgFile, + response: `{"ok":true}`, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + fs := afero.NewMemMapFs() + if tc.setupFs != nil { + tc.setupFs(t, fs) + } + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appIconMethod, + Response: tc.response, + }) + defer teardown() + _, err := c.Icon(ctx, fs, "token", tc.appID, tc.filePath) + if tc.expectErr { + require.Error(t, err) + if tc.errContains != "" { + require.Contains(t, err.Error(), tc.errContains) + } + } else { + require.NoError(t, err) + } + }) + } +} + +func TestClient_IconSet(t *testing.T) { + tests := map[string]struct { + setupFs func(t *testing.T, fs afero.Fs) + appID string + filePath string + response string + expectErr bool + errContains string + }{ + "returns error when args are missing": { + appID: "", + filePath: "", + expectErr: true, + errContains: "missing required args", + }, + "returns error when file does not exist": { + appID: "12345", + filePath: imgFile, + expectErr: true, + errContains: "file does not exist", + }, + "returns error for empty file": { + setupFs: func(t *testing.T, fs afero.Fs) { + err := afero.WriteFile(fs, imgFile, []byte{}, 0666) + require.NoError(t, err) + }, + appID: "12345", + filePath: imgFile, + expectErr: true, + }, + "returns error for unsupported format": { + setupFs: func(t *testing.T, fs afero.Fs) { + svgContent := []byte(``) + err := afero.WriteFile(fs, "icon.svg", svgContent, 0666) + require.NoError(t, err) + }, + appID: "12345", + filePath: "icon.svg", + expectErr: true, + errContains: "unknown format", + }, + "returns error for non-image content with .png extension": { + setupFs: func(t *testing.T, fs afero.Fs) { + err := afero.WriteFile(fs, "fake.png", []byte("this is not a png"), 0666) + require.NoError(t, err) + }, + appID: "12345", + filePath: "fake.png", + expectErr: true, + errContains: "unknown format", + }, + "returns error for truncated PNG": { + setupFs: func(t *testing.T, fs afero.Fs) { + // Valid PNG signature followed by incomplete IHDR chunk + truncatedPNG := []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x00, 0x00, 0x00, 0x0D} + err := afero.WriteFile(fs, "truncated.png", truncatedPNG, 0666) + require.NoError(t, err) + }, + appID: "12345", + filePath: "truncated.png", + expectErr: true, + }, + "returns error from API response": { + setupFs: func(t *testing.T, fs afero.Fs) { + createTestPNG(t, fs, imgFile) + }, + appID: "12345", + filePath: imgFile, + response: `{"ok":false,"error":"invalid_app"}`, + expectErr: true, + errContains: "invalid_app", + }, + "succeeds with valid PNG": { + setupFs: func(t *testing.T, fs afero.Fs) { + createTestPNG(t, fs, imgFile) + }, + appID: "12345", + filePath: imgFile, + response: `{"ok":true}`, + }, + // cutter.Crop with Ratio mode floors a 1x1 image to 0x0, causing png.Encode to fail. + // May need a follow-up to add a minimum dimension check before cropping. + "returns error for 1x1 pixel image": { + setupFs: func(t *testing.T, fs afero.Fs) { + myimage := image.NewRGBA(image.Rectangle{image.Point{0, 0}, image.Point{1, 1}}) + myimage.Set(0, 0, color.RGBA{255, 0, 0, 255}) + myfile, err := fs.Create(imgFile) + require.NoError(t, err) + err = png.Encode(myfile, myimage) + require.NoError(t, err) + }, + appID: "12345", + filePath: imgFile, + expectErr: true, + }, + "succeeds with non-square image": { + setupFs: func(t *testing.T, fs afero.Fs) { + myimage := image.NewRGBA(image.Rectangle{image.Point{0, 0}, image.Point{2000, 100}}) + for x := range 2000 { + for y := range 100 { + c := color.RGBA{uint8(rand.Intn(255)), uint8(rand.Intn(255)), uint8(rand.Intn(255)), 255} + myimage.Set(x, y, c) + } + } + myfile, err := fs.Create(imgFile) + require.NoError(t, err) + err = png.Encode(myfile, myimage) + require.NoError(t, err) + }, + appID: "12345", + filePath: imgFile, + response: `{"ok":true}`, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + fs := afero.NewMemMapFs() + if tc.setupFs != nil { + tc.setupFs(t, fs) + } + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appIconSetMethod, + Response: tc.response, + }) + defer teardown() + _, err := c.IconSet(ctx, fs, "token", tc.appID, tc.filePath) + if tc.expectErr { + require.Error(t, err) + if tc.errContains != "" { + require.Contains(t, err.Error(), tc.errContains) + } + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/internal/experiment/experiment.go b/internal/experiment/experiment.go index 7d51b654..80c661f9 100644 --- a/internal/experiment/experiment.go +++ b/internal/experiment/experiment.go @@ -38,6 +38,9 @@ const ( // Sandboxes experiment lets users who have joined the Slack Developer Program use the CLI to manage their sandboxes. Sandboxes Experiment = "sandboxes" + + // SetIcon experiment enables icon upload for non-hosted apps. + SetIcon Experiment = "set-icon" ) // AllExperiments is a list of all available experiments that can be enabled @@ -46,6 +49,7 @@ var AllExperiments = []Experiment{ Lipgloss, Placeholder, Sandboxes, + SetIcon, } // EnabledExperiments is a list of experiments that are permanently enabled diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 7cf505b9..1e3ee7df 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -24,6 +24,7 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/api" "github.com/slackapi/slack-cli/internal/config" + "github.com/slackapi/slack-cli/internal/experiment" "github.com/slackapi/slack-cli/internal/pkg/manifest" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" @@ -521,30 +522,25 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran return app, result, installState, err } - // - // TODO: Currently, cannot update the icon if app is not hosted. - // - // upload icon, default to icon.png - // var iconPath = slackYaml.Icon - // if iconPath == "" { - // if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { - // iconPath = "icon.png" - // } - // } - // if iconPath != "" { - // clients.IO.PrintDebug(ctx, "uploading icon") - // err = updateIcon(ctx, clients, iconPath, env.AppID, token) - // if err != nil { - // clients.IO.PrintError(ctx, "An error occurred updating the Icon", err) - // } - // // Save a md5 hash of the icon in environments.yaml - // var iconHash string - // iconHash, err = getIconHash(iconPath) - // if err != nil { - // return env, api.DeveloperAppInstallResult{}, err - // } - // env.IconHash = iconHash - // } + // upload icon for non-hosted apps (gated behind set-icon experiment) + if clients.Config.WithExperimentOn(experiment.SetIcon) { + var iconPath = slackManifest.Icon + if iconPath == "" { + if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { + iconPath = "icon.png" + } + } + if iconPath != "" { + clients.IO.PrintDebug(ctx, "uploading icon") + _, iconErr := clients.API().IconSet(ctx, clients.Fs, token, app.AppID, iconPath) + if iconErr != nil { + clients.IO.PrintDebug(ctx, "icon error: %s", iconErr) + _, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Error updating app icon: %s", iconErr))) + } else { + _, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Updated app icon: %s", iconPath))) + } + } + } // update config with latest yaml hash // env.Hash = slackYaml.Hash @@ -662,9 +658,12 @@ func updateIcon(ctx context.Context, clients *shared.ClientFactory, iconPath, ap clients.IO.PrintDebug(ctx, "uploading icon") - // var iconResp apiclient.IconResult var err error - _, err = clients.API().Icon(ctx, clients.Fs, token, appID, iconPath) + if clients.Config.WithExperimentOn(experiment.SetIcon) { + _, err = clients.API().IconSet(ctx, clients.Fs, token, appID, iconPath) + } else { + _, err = clients.API().Icon(ctx, clients.Fs, token, appID, iconPath) + } if err != nil { // TODO: separate the icon upload into a different function because if an error is returned // the new app_id might be ignored and next time we'll create another app.