Skip to content

Commit bef3a23

Browse files
srtaalejzimeg
srtaalej
and
zimeg
authored
feat: support SLACK_CLI_APP_ICON_PATH env var for icon override (#519)
Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>
1 parent d0dcd8c commit bef3a23

7 files changed

Lines changed: 138 additions & 20 deletions

File tree

‎internal/config/config.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
// Environment Variable constants
2525
const slackAccessibleEnv = "ACCESSIBLE"
2626
const slackAutoRequestAAAEnv = "SLACK_AUTO_REQUEST_AAA"
27+
const slackCLIAppIconPathEnv = "SLACK_CLI_APP_ICON_PATH"
2728
const slackConfigDirEnv = "SLACK_CONFIG_DIR"
2829
const slackDisableTelemetryEnv = "SLACK_DISABLE_TELEMETRY"
2930
const slackTestTraceEnv = "SLACK_TEST_TRACE"
@@ -43,6 +44,7 @@ type Config struct {
4344
APIHostFlag string
4445
APIHostResolved string
4546
AppFlag string
47+
AppIconPathFlag string
4648
AutoRequestAAAFlag bool
4749
ConfigDirFlag string
4850
DebugEnabled bool

‎internal/config/dotenv.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ func (c *Config) LoadEnvironmentVariables() error {
5151
c.ConfigDirFlag = configDir
5252
}
5353

54+
// Load app icon path from environment variables
55+
var appIconPath = strings.TrimSpace(c.os.Getenv(slackCLIAppIconPathEnv))
56+
if appIconPath != "" {
57+
c.AppIconPathFlag = appIconPath
58+
}
59+
5460
// Disable telemetry if either disable-telemetry or test-version environment variables
5561
var disableTelemetry = strings.TrimSpace(c.os.Getenv(slackDisableTelemetryEnv))
5662
var testVersion = strings.TrimSpace(c.os.Getenv(version.EnvTestVersion))

‎internal/config/dotenv_test.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,27 @@ func Test_DotEnv_LoadEnvironmentVariables(t *testing.T) {
153153
assert.Equal(t, false, cfg.AutoRequestAAAFlag)
154154
},
155155
},
156+
"SLACK_CLI_APP_ICON_PATH=/path/to/icon.png should set AppIconPathFlag": {
157+
envName: "SLACK_CLI_APP_ICON_PATH",
158+
envValue: "/path/to/icon.png",
159+
assertOnConfig: func(t *testing.T, cfg *Config) {
160+
assert.Equal(t, "/path/to/icon.png", cfg.AppIconPathFlag)
161+
},
162+
},
163+
"SLACK_CLI_APP_ICON_PATH= should not set AppIconPathFlag": {
164+
envName: "SLACK_CLI_APP_ICON_PATH",
165+
envValue: "",
166+
assertOnConfig: func(t *testing.T, cfg *Config) {
167+
assert.Equal(t, "", cfg.AppIconPathFlag)
168+
},
169+
},
170+
"SLACK_CLI_APP_ICON_PATH with whitespace should be trimmed": {
171+
envName: "SLACK_CLI_APP_ICON_PATH",
172+
envValue: " /path/to/icon.png ",
173+
assertOnConfig: func(t *testing.T, cfg *Config) {
174+
assert.Equal(t, "/path/to/icon.png", cfg.AppIconPathFlag)
175+
},
176+
},
156177
"SLACK_CONFIG_DIR=/path/to/config should set the config dir": {
157178
envName: "SLACK_CONFIG_DIR",
158179
envValue: "/path/to/config",

‎internal/icon/icon.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,7 @@ import (
2020
"github.com/spf13/afero"
2121
)
2222

23-
func ResolveIconPath(fs afero.Fs, manifestIcon string) string {
24-
if manifestIcon != "" {
25-
return manifestIcon
26-
}
23+
func ResolveIconPath(fs afero.Fs) string {
2724
supportedExtensions := []string{".png", ".jpg", ".jpeg", ".gif"}
2825
for _, dir := range []string{"assets", "."} {
2926
for _, ext := range supportedExtensions {

‎internal/icon/icon_test.go‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,9 @@ import (
2424

2525
func Test_ResolveIconPath(t *testing.T) {
2626
tests := map[string]struct {
27-
manifestIcon string
28-
files []string
29-
expected string
27+
files []string
28+
expected string
3029
}{
31-
"manifest icon set returns it directly": {
32-
manifestIcon: "custom/my-icon.png",
33-
expected: "custom/my-icon.png",
34-
},
35-
"manifest icon preferred over magic icon files": {
36-
manifestIcon: "custom/my-icon.png",
37-
files: []string{"assets/icon.png", "icon.png"},
38-
expected: "custom/my-icon.png",
39-
},
4030
"assets/icon.png found": {
4131
files: []string{"assets/icon.png"},
4232
expected: "assets/icon.png",
@@ -84,7 +74,7 @@ func Test_ResolveIconPath(t *testing.T) {
8474
for _, f := range tc.files {
8575
require.NoError(t, afero.WriteFile(fs, f, []byte("img"), 0o644))
8676
}
87-
result := ResolveIconPath(fs, tc.manifestIcon)
77+
result := ResolveIconPath(fs)
8878
assert.Equal(t, tc.expected, result)
8979
})
9080
}

‎internal/pkg/apps/install.go‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,7 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac
218218
}
219219
}
220220

221-
// upload icon, default to assets/icon.{png,jpg,jpeg,gif} or icon.{png,jpg,jpeg,gif}
222-
iconPath := icon.ResolveIconPath(clients.Fs, slackManifest.Icon)
221+
iconPath := resolveIconPath(ctx, clients, slackManifest.Icon)
223222
if iconPath != "" {
224223
err = updateIcon(ctx, clients, iconPath, app.AppID, token, manifest.IsFunctionRuntimeSlackHosted())
225224
if err != nil {
@@ -519,7 +518,7 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran
519518

520519
// upload icon for non-hosted apps (gated behind set-icon experiment)
521520
if clients.Config.WithExperimentOn(experiment.SetIcon) {
522-
iconPath := icon.ResolveIconPath(clients.Fs, slackManifest.Icon)
521+
iconPath := resolveIconPath(ctx, clients, slackManifest.Icon)
523522
if iconPath != "" {
524523
_, iconErr := clients.API().IconSet(ctx, clients.Fs, token, app.AppID, iconPath)
525524
if iconErr != nil {
@@ -639,6 +638,28 @@ func appendLocalToDisplayName(manifest *types.AppManifest) {
639638
}
640639
}
641640

641+
// resolveIconPath determines the icon file path using the priority chain:
642+
// SLACK_CLI_APP_ICON_PATH env var > manifest icon field > assets/ and root fallback
643+
func resolveIconPath(ctx context.Context, clients *shared.ClientFactory, manifestIcon string) string {
644+
if envIconPath := clients.Config.AppIconPathFlag; envIconPath != "" {
645+
if _, err := clients.Fs.Stat(envIconPath); err == nil {
646+
return envIconPath
647+
}
648+
clients.IO.PrintDebug(ctx, "SLACK_CLI_APP_ICON_PATH file not found: %s", envIconPath)
649+
_, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Warning: icon path from SLACK_CLI_APP_ICON_PATH not found: %s", envIconPath)))
650+
return ""
651+
}
652+
if manifestIcon != "" {
653+
if _, err := clients.Fs.Stat(manifestIcon); err == nil {
654+
return manifestIcon
655+
}
656+
clients.IO.PrintDebug(ctx, "manifest icon file not found: %s", manifestIcon)
657+
_, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Warning: icon path from manifest not found: %s", manifestIcon)))
658+
return ""
659+
}
660+
return icon.ResolveIconPath(clients.Fs)
661+
}
662+
642663
// updateIcon will upload the new icon to the Slack API
643664
func updateIcon(ctx context.Context, clients *shared.ClientFactory, iconPath, appID string, token string, isHosted bool) error {
644665
var span opentracing.Span

‎internal/pkg/apps/install_test.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/slackapi/slack-cli/internal/shared/types"
3030
"github.com/slackapi/slack-cli/internal/slackcontext"
3131
"github.com/slackapi/slack-cli/internal/slackerror"
32+
"github.com/spf13/afero"
3233
"github.com/stretchr/testify/assert"
3334
"github.com/stretchr/testify/mock"
3435
"github.com/stretchr/testify/require"
@@ -1728,6 +1729,86 @@ func TestContinueDespiteWarning(t *testing.T) {
17281729
}
17291730
}
17301731

1732+
func Test_resolveIconPath(t *testing.T) {
1733+
tests := map[string]struct {
1734+
envIconPath string
1735+
manifestIcon string
1736+
setupFiles func(t *testing.T, fs afero.Fs)
1737+
expected string
1738+
}{
1739+
"env var takes priority over manifest icon": {
1740+
envIconPath: "env-icon.png",
1741+
manifestIcon: "manifest-icon.png",
1742+
setupFiles: func(t *testing.T, fs afero.Fs) {
1743+
require.NoError(t, afero.WriteFile(fs, "env-icon.png", []byte("img"), 0o644))
1744+
require.NoError(t, afero.WriteFile(fs, "manifest-icon.png", []byte("img"), 0o644))
1745+
},
1746+
expected: "env-icon.png",
1747+
},
1748+
"env var takes priority over fallback": {
1749+
envIconPath: "env-icon.png",
1750+
setupFiles: func(t *testing.T, fs afero.Fs) {
1751+
require.NoError(t, afero.WriteFile(fs, "env-icon.png", []byte("img"), 0o644))
1752+
require.NoError(t, afero.WriteFile(fs, "assets/icon.png", []byte("img"), 0o644))
1753+
},
1754+
expected: "env-icon.png",
1755+
},
1756+
"manifest icon used when no env var": {
1757+
manifestIcon: "manifest-icon.png",
1758+
setupFiles: func(t *testing.T, fs afero.Fs) {
1759+
require.NoError(t, afero.WriteFile(fs, "manifest-icon.png", []byte("img"), 0o644))
1760+
},
1761+
expected: "manifest-icon.png",
1762+
},
1763+
"falls back to assets/icon.png when no env var or manifest": {
1764+
setupFiles: func(t *testing.T, fs afero.Fs) {
1765+
require.NoError(t, afero.WriteFile(fs, "assets/icon.png", []byte("img"), 0o644))
1766+
},
1767+
expected: "assets/icon.png",
1768+
},
1769+
"falls back to icon.png in root when no assets": {
1770+
setupFiles: func(t *testing.T, fs afero.Fs) {
1771+
require.NoError(t, afero.WriteFile(fs, "icon.png", []byte("img"), 0o644))
1772+
},
1773+
expected: "icon.png",
1774+
},
1775+
"returns empty when no icon found": {
1776+
setupFiles: func(t *testing.T, fs afero.Fs) {},
1777+
expected: "",
1778+
},
1779+
"env var file not found returns empty": {
1780+
envIconPath: "missing-icon.png",
1781+
manifestIcon: "manifest-icon.png",
1782+
setupFiles: func(t *testing.T, fs afero.Fs) {
1783+
require.NoError(t, afero.WriteFile(fs, "manifest-icon.png", []byte("img"), 0o644))
1784+
},
1785+
expected: "",
1786+
},
1787+
"manifest icon file not found returns empty with warning": {
1788+
manifestIcon: "missing-manifest-icon.png",
1789+
setupFiles: func(t *testing.T, fs afero.Fs) {
1790+
require.NoError(t, afero.WriteFile(fs, "assets/icon.png", []byte("img"), 0o644))
1791+
},
1792+
expected: "",
1793+
},
1794+
}
1795+
for name, tc := range tests {
1796+
t.Run(name, func(t *testing.T) {
1797+
ctx := slackcontext.MockContext(t.Context())
1798+
clientsMock := shared.NewClientsMock()
1799+
clientsMock.AddDefaultMocks()
1800+
clientsMock.Config.AppIconPathFlag = tc.envIconPath
1801+
tc.setupFiles(t, clientsMock.Fs)
1802+
output := &bytes.Buffer{}
1803+
clientsMock.IO.Stdout.SetOutput(output)
1804+
clients := shared.NewClientFactory(clientsMock.MockClientFactory())
1805+
1806+
result := resolveIconPath(ctx, clients, tc.manifestIcon)
1807+
assert.Equal(t, tc.expected, result)
1808+
})
1809+
}
1810+
}
1811+
17311812
func Test_updateIcon(t *testing.T) {
17321813
tests := map[string]struct {
17331814
isHosted bool

0 commit comments

Comments
 (0)