Skip to content

feat: support icon path in project config.json #514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
srtaalej wants to merge 1 commit into from
Closed

feat: support icon path in project config.json #514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions cmd/app/add_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
},
Expand All @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand Down
34 changes: 34 additions & 0 deletions internal/config/project.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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"`
Expand All @@ -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) {
Expand Down
11 changes: 11 additions & 0 deletions internal/config/project_mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
101 changes: 101 additions & 0 deletions internal/config/project_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
10 changes: 7 additions & 3 deletions internal/pkg/apps/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
Expand Down Expand Up @@ -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"
}
}
Expand Down
4 changes: 4 additions & 0 deletions internal/pkg/apps/install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
Loading