From 54bc2bbef5e130fb6860e8448b285ed6d288ea2a Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Wed, 29 Apr 2026 11:29:19 -0400 Subject: [PATCH] feat: support icon path in project config.json Allow users to specify an icon path in .slack/config.json as an alternative to the manifest icon field. Resolution order is: manifest > config.json > icon.png in project root. Also migrates tests to use AddDefaultMocks() for ProjectConfigMock instead of manually setting up each mock method. --- cmd/app/add_test.go | 25 ++++---- internal/config/project.go | 34 ++++++++++ internal/config/project_mock.go | 11 ++++ internal/config/project_test.go | 101 ++++++++++++++++++++++++++++++ internal/pkg/apps/install.go | 10 ++- internal/pkg/apps/install_test.go | 4 ++ 6 files changed, 171 insertions(+), 14 deletions(-) diff --git a/cmd/app/add_test.go b/cmd/app/add_test.go index a7ecddde..7ee78df9 100644 --- a/cmd/app/add_test.go +++ b/cmd/app/add_test.go @@ -90,7 +90,7 @@ func TestAppAddCommandPreRun(t *testing.T) { cf.SDKConfig.WorkingDirectory = "." cm.AddDefaultMocks() mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() cm.Config.ProjectConfig = mockProjectConfig }, }, @@ -100,6 +100,9 @@ func TestAppAddCommandPreRun(t *testing.T) { cf.SDKConfig.WorkingDirectory = "." cm.AddDefaultMocks() mockProjectConfig := config.NewProjectConfigMock() + mockProjectConfig.AddDefaultMocks() + // Override default ManifestSourceLocal with Remote for this test + mockProjectConfig.On("GetManifestSource", mock.Anything).Unset() mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceRemote, nil) cm.Config.ProjectConfig = mockProjectConfig }, @@ -202,7 +205,7 @@ func TestAppAddCommand(t *testing.T) { mockProjectCache.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything). Return(nil) mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() mockProjectConfig.On("Cache").Return(mockProjectCache) cm.Config.ProjectConfig = mockProjectConfig }, @@ -275,7 +278,7 @@ func TestAppAddCommand(t *testing.T) { mockProjectCache.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything). Return(nil) mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() mockProjectConfig.On("Cache").Return(mockProjectCache) cm.Config.ProjectConfig = mockProjectConfig }, @@ -358,7 +361,7 @@ func TestAppAddCommand(t *testing.T) { mockProjectCache.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything). Return(nil) mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() mockProjectConfig.On("Cache").Return(mockProjectCache) cm.Config.ProjectConfig = mockProjectConfig }, @@ -426,7 +429,7 @@ func TestAppAddCommand(t *testing.T) { mockProjectCache.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything). Return(nil) mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() mockProjectConfig.On("Cache").Return(mockProjectCache) cm.Config.ProjectConfig = mockProjectConfig }, @@ -514,7 +517,7 @@ func TestAppAddCommand(t *testing.T) { mockProjectCache.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything). Return(nil) mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() mockProjectConfig.On("Cache").Return(mockProjectCache) cm.Config.ProjectConfig = mockProjectConfig }, @@ -599,7 +602,7 @@ func TestAppAddCommand(t *testing.T) { mockProjectCache.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything). Return(nil) mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() mockProjectConfig.On("Cache").Return(mockProjectCache) cm.Config.ProjectConfig = mockProjectConfig }, @@ -686,7 +689,7 @@ func TestAppAddCommand(t *testing.T) { mockProjectCache.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything). Return(nil) mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() mockProjectConfig.On("Cache").Return(mockProjectCache) cm.Config.ProjectConfig = mockProjectConfig }, @@ -762,7 +765,7 @@ func TestAppAddCommand(t *testing.T) { mockProjectCache.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything). Return(nil) mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() mockProjectConfig.On("Cache").Return(mockProjectCache) cm.Config.ProjectConfig = mockProjectConfig }, @@ -819,7 +822,7 @@ func TestAppAddCommand(t *testing.T) { mockProjectCache.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything). Return(nil) mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() mockProjectConfig.On("Cache").Return(mockProjectCache) cm.Config.ProjectConfig = mockProjectConfig }, @@ -876,7 +879,7 @@ func TestAppAddCommand(t *testing.T) { mockProjectCache.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything). Return(nil) mockProjectConfig := config.NewProjectConfigMock() - mockProjectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + mockProjectConfig.AddDefaultMocks() mockProjectConfig.On("Cache").Return(mockProjectCache) cm.Config.ProjectConfig = mockProjectConfig }, diff --git a/internal/config/project.go b/internal/config/project.go index c2c03be1..371cfedf 100644 --- a/internal/config/project.go +++ b/internal/config/project.go @@ -47,6 +47,8 @@ var dotGitignoreFileData []byte // ProjectConfigManager is the interface for interacting with the project config type ProjectConfigManager interface { + GetIconPath(ctx context.Context) (string, error) + SetIconPath(ctx context.Context, iconPath string) error InitProjectID(ctx context.Context, overwriteExistingProjectID bool) (string, error) GetProjectID(ctx context.Context) (string, error) SetProjectID(ctx context.Context, projectID string) (string, error) @@ -60,6 +62,7 @@ type ProjectConfigManager interface { // ProjectConfig is the project-level config file type ProjectConfig struct { Experiments map[string]bool `json:"experiments,omitempty"` + Icon string `json:"icon,omitempty"` Manifest *ManifestConfig `json:"manifest,omitempty"` ProjectID string `json:"project_id,omitempty"` Surveys map[string]SurveyConfig `json:"surveys,omitempty"` @@ -81,6 +84,37 @@ func NewProjectConfig(fs afero.Fs, os types.Os) *ProjectConfig { return projectConfig } +// GetIconPath reads the icon path from the project-level config file +func (c *ProjectConfig) GetIconPath(ctx context.Context) (string, error) { + var span opentracing.Span + span, ctx = opentracing.StartSpanFromContext(ctx, "GetIconPath") + defer span.Finish() + + var projectConfig, err = ReadProjectConfigFile(ctx, c.fs, c.os) + if err != nil { + return "", err + } + + return strings.TrimSpace(projectConfig.Icon), nil +} + +// SetIconPath sets the icon path in the project-level config file +func (c *ProjectConfig) SetIconPath(ctx context.Context, iconPath string) error { + var span opentracing.Span + span, ctx = opentracing.StartSpanFromContext(ctx, "SetIconPath") + defer span.Finish() + + var projectConfig, err = ReadProjectConfigFile(ctx, c.fs, c.os) + if err != nil { + return err + } + + projectConfig.Icon = iconPath + + _, err = WriteProjectConfigFile(ctx, c.fs, c.os, projectConfig) + return err +} + // InitProjectID will set the project_id in the project-level config when it's unset // and then returns the project_id. func (c *ProjectConfig) InitProjectID(ctx context.Context, overwriteExistingProjectID bool) (string, error) { diff --git a/internal/config/project_mock.go b/internal/config/project_mock.go index 37e4778d..6a9c0019 100644 --- a/internal/config/project_mock.go +++ b/internal/config/project_mock.go @@ -30,9 +30,20 @@ func NewProjectConfigMock() *ProjectConfigMock { } func (m *ProjectConfigMock) AddDefaultMocks() { + m.On("GetIconPath", mock.Anything).Return("", nil) m.On("GetManifestSource", mock.Anything).Return(ManifestSourceLocal, nil) } +func (m *ProjectConfigMock) GetIconPath(ctx context.Context) (string, error) { + args := m.Called(ctx) + return args.String(0), args.Error(1) +} + +func (m *ProjectConfigMock) SetIconPath(ctx context.Context, iconPath string) error { + args := m.Called(ctx, iconPath) + return args.Error(0) +} + func (m *ProjectConfigMock) InitProjectID(ctx context.Context, overwriteExistingProjectID bool) (string, error) { args := m.Called(ctx, overwriteExistingProjectID) return args.String(0), args.Error(1) diff --git a/internal/config/project_test.go b/internal/config/project_test.go index 7c3f1573..54f44c21 100644 --- a/internal/config/project_test.go +++ b/internal/config/project_test.go @@ -139,6 +139,107 @@ func Test_ProjectConfig_InitProjectID(t *testing.T) { }) } +func Test_ProjectConfig_GetIconPath(t *testing.T) { + t.Run("When not a project directory, should return an error", func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + fs := slackdeps.NewFsMock() + os := slackdeps.NewOsMock() + os.AddDefaultMocks() + + projectConfig := NewProjectConfig(fs, os) + iconPath, err := projectConfig.GetIconPath(ctx) + + require.Error(t, err) + require.Empty(t, iconPath) + }) + + t.Run("When icon is not set, should return empty string", func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + fs := slackdeps.NewFsMock() + os := slackdeps.NewOsMock() + os.AddDefaultMocks() + addProjectMocks(t, fs) + + projectConfig := NewProjectConfig(fs, os) + _, err := WriteProjectConfigFile(ctx, fs, os, ProjectConfig{ProjectID: "test-123"}) + require.NoError(t, err) + + iconPath, err := projectConfig.GetIconPath(ctx) + + require.NoError(t, err) + require.Empty(t, iconPath) + }) + + t.Run("When icon is set, should return trimmed icon path", func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + fs := slackdeps.NewFsMock() + os := slackdeps.NewOsMock() + os.AddDefaultMocks() + addProjectMocks(t, fs) + + projectConfig := NewProjectConfig(fs, os) + _, err := WriteProjectConfigFile(ctx, fs, os, ProjectConfig{Icon: " assets/icon.png "}) + require.NoError(t, err) + + iconPath, err := projectConfig.GetIconPath(ctx) + + require.NoError(t, err) + require.Equal(t, "assets/icon.png", iconPath) + }) +} + +func Test_ProjectConfig_SetIconPath(t *testing.T) { + t.Run("When not a project directory, should return an error", func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + fs := slackdeps.NewFsMock() + os := slackdeps.NewOsMock() + os.AddDefaultMocks() + + projectConfig := NewProjectConfig(fs, os) + err := projectConfig.SetIconPath(ctx, "icon.png") + + require.Error(t, err) + }) + + t.Run("When a project directory, should update the icon path", func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + fs := slackdeps.NewFsMock() + os := slackdeps.NewOsMock() + os.AddDefaultMocks() + addProjectMocks(t, fs) + + projectConfig := NewProjectConfig(fs, os) + _, err := WriteProjectConfigFile(ctx, fs, os, ProjectConfig{ProjectID: "test-123"}) + require.NoError(t, err) + + err = projectConfig.SetIconPath(ctx, "images/my-icon.png") + require.NoError(t, err) + + iconPath, err := projectConfig.GetIconPath(ctx) + require.NoError(t, err) + require.Equal(t, "images/my-icon.png", iconPath) + }) + + t.Run("should preserve existing config fields", func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + fs := slackdeps.NewFsMock() + os := slackdeps.NewOsMock() + os.AddDefaultMocks() + addProjectMocks(t, fs) + + projectConfig := NewProjectConfig(fs, os) + _, err := WriteProjectConfigFile(ctx, fs, os, ProjectConfig{ProjectID: "test-123"}) + require.NoError(t, err) + + err = projectConfig.SetIconPath(ctx, "icon.png") + require.NoError(t, err) + + projectID, err := projectConfig.GetProjectID(ctx) + require.NoError(t, err) + require.Equal(t, "test-123", projectID) + }) +} + func Test_ProjectConfig_GetProjectID(t *testing.T) { t.Run("When not a project directory, should return an error", func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 9dcd68ee..45572a88 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -218,10 +218,12 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac } } - // upload icon, default to icon.png + // upload icon — check manifest, then config.json, then icon.png in project root var iconPath = slackManifest.Icon if iconPath == "" { - if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { + if configIcon, err := clients.Config.ProjectConfig.GetIconPath(ctx); err == nil && configIcon != "" { + iconPath = configIcon + } else if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { iconPath = "icon.png" } } @@ -526,7 +528,9 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran if clients.Config.WithExperimentOn(experiment.SetIcon) { var iconPath = slackManifest.Icon if iconPath == "" { - if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { + if configIcon, err := clients.Config.ProjectConfig.GetIconPath(ctx); err == nil && configIcon != "" { + iconPath = configIcon + } else if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { iconPath = "icon.png" } } diff --git a/internal/pkg/apps/install_test.go b/internal/pkg/apps/install_test.go index 2c44dc94..a914a6a6 100644 --- a/internal/pkg/apps/install_test.go +++ b/internal/pkg/apps/install_test.go @@ -586,6 +586,8 @@ func TestInstall(t *testing.T) { manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything).Return(tc.mockManifestAppRemote, nil) clientsMock.AppClient.Manifest = manifestMock mockProjectConfig := config.NewProjectConfigMock() + mockProjectConfig.AddDefaultMocks() + mockProjectConfig.On("GetManifestSource", mock.Anything).Unset() mockProjectConfig.On("GetManifestSource", mock.Anything).Return(tc.mockManifestSource, nil) mockProjectCache := cache.NewCacheMock() mockProjectCache.On( @@ -1408,6 +1410,8 @@ func TestInstallLocalApp(t *testing.T) { manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything).Return(tc.mockManifest, nil) clientsMock.AppClient.Manifest = manifestMock mockProjectConfig := config.NewProjectConfigMock() + mockProjectConfig.AddDefaultMocks() + mockProjectConfig.On("GetManifestSource", mock.Anything).Unset() mockProjectConfig.On("GetManifestSource", mock.Anything).Return(tc.mockManifestSource, nil) mockProjectCache := cache.NewCacheMock() mockProjectCache.On(