From 83f47e1b5de18fdda95353704bda7fa169a48f23 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 5 Mar 2026 14:52:13 -0800 Subject: [PATCH 1/5] refactor: remove logger from activity, validate, datastore, auth list, delete, and uninstall --- cmd/app/delete.go | 31 ++++----------- cmd/app/uninstall.go | 31 ++++----------- cmd/auth/auth_test.go | 3 +- cmd/auth/list.go | 24 +---------- cmd/auth/list_test.go | 3 +- cmd/datastore/bulk_delete.go | 29 ++------------ cmd/datastore/bulk_delete_test.go | 13 +++--- cmd/datastore/bulk_get.go | 30 ++------------ cmd/datastore/bulk_get_test.go | 13 +++--- cmd/datastore/bulk_put.go | 30 ++------------ cmd/datastore/bulk_put_test.go | 13 +++--- cmd/datastore/delete.go | 29 ++------------ cmd/datastore/delete_test.go | 13 +++--- cmd/datastore/get.go | 30 ++------------ cmd/datastore/get_test.go | 13 +++--- cmd/datastore/put.go | 30 ++------------ cmd/datastore/put_test.go | 13 +++--- cmd/datastore/query.go | 30 ++------------ cmd/datastore/query_test.go | 13 +++--- cmd/datastore/update.go | 30 ++------------ cmd/datastore/update_test.go | 13 +++--- cmd/manifest/validate.go | 46 +++++++--------------- cmd/manifest/validate_test.go | 16 ++++---- cmd/platform/activity.go | 17 +------- cmd/platform/activity_test.go | 2 - internal/pkg/apps/delete.go | 24 ++++------- internal/pkg/apps/delete_test.go | 3 +- internal/pkg/apps/uninstall.go | 22 ++++------- internal/pkg/auth/list.go | 8 +--- internal/pkg/auth/list_test.go | 5 +-- internal/pkg/datastore/bulk_delete.go | 11 ++---- internal/pkg/datastore/bulk_delete_test.go | 8 +--- internal/pkg/datastore/bulk_get.go | 11 ++---- internal/pkg/datastore/bulk_get_test.go | 8 +--- internal/pkg/datastore/bulk_put.go | 11 ++---- internal/pkg/datastore/bulk_put_test.go | 8 +--- internal/pkg/datastore/delete.go | 11 ++---- internal/pkg/datastore/delete_test.go | 8 +--- internal/pkg/datastore/get.go | 11 ++---- internal/pkg/datastore/get_test.go | 8 +--- internal/pkg/datastore/put.go | 11 ++---- internal/pkg/datastore/put_test.go | 8 +--- internal/pkg/datastore/query.go | 10 ++--- internal/pkg/datastore/query_test.go | 8 +--- internal/pkg/datastore/update.go | 11 ++---- internal/pkg/datastore/update_test.go | 8 +--- internal/pkg/manifest/validate.go | 16 ++++---- internal/pkg/manifest/validate_test.go | 41 +++++++++---------- internal/pkg/platform/activity.go | 2 - internal/pkg/platform/activity_test.go | 3 +- internal/pkg/platform/localserver.go | 2 +- internal/pkg/platform/run.go | 2 +- 52 files changed, 201 insertions(+), 593 deletions(-) diff --git a/cmd/app/delete.go b/cmd/app/delete.go index 2ae6ae43..30d3264b 100644 --- a/cmd/app/delete.go +++ b/cmd/app/delete.go @@ -20,7 +20,6 @@ import ( "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/apps" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -99,30 +98,14 @@ func RunDeleteCommand(ctx context.Context, clients *shared.ClientFactory, cmd *c } } - // Set up event logger and execute the command - log := newDeleteLogger(clients, cmd, team) - log.Data["appID"] = selection.App.AppID - env, err := apps.Delete(ctx, clients, log, team, selection.App, selection.Auth) - - return env, err -} + // Execute the command + env, teamName, err := apps.Delete(ctx, clients, team, selection.App, selection.Auth) + if err != nil { + return env, err + } + printDeleteApp(ctx, clients, selection.App.AppID, teamName) -// newDeleteLogger creates a logger instance to receive event notifications -func newDeleteLogger(clients *shared.ClientFactory, cmd *cobra.Command, envName string) *logger.Logger { - ctx := cmd.Context() - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - appID := event.DataToString("appID") - teamName := event.DataToString("teamName") - switch event.Name { - case "on_apps_delete_app_success": - printDeleteApp(ctx, clients, appID, teamName) - default: - // Ignore the event - } - }, - ) + return env, nil } func confirmDeletion(ctx context.Context, IO iostreams.IOStreamer, app prompts.SelectedApp) (bool, error) { diff --git a/cmd/app/uninstall.go b/cmd/app/uninstall.go index 9fca5027..6fde4ec9 100644 --- a/cmd/app/uninstall.go +++ b/cmd/app/uninstall.go @@ -20,7 +20,6 @@ import ( "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/apps" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -94,30 +93,14 @@ func RunUninstallCommand(ctx context.Context, clients *shared.ClientFactory, cmd } } - // Set up event logger and execute the command - log := newUninstallLogger(clients, cmd, teamDomain) - log.Data["appID"] = selection.App.AppID - env, err := apps.Uninstall(ctx, clients, log, teamDomain, selection.App, selection.Auth) - - return env, err -} + // Execute the command + env, teamName, err := apps.Uninstall(ctx, clients, teamDomain, selection.App, selection.Auth) + if err != nil { + return env, err + } + printUninstallApp(ctx, clients, selection.App.AppID, teamName) -// newUninstallLogger creates a logger instance to receive event notifications -func newUninstallLogger(clients *shared.ClientFactory, cmd *cobra.Command, envName string) *logger.Logger { - ctx := cmd.Context() - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - appID := event.DataToString("appID") - teamName := event.DataToString("teamName") - switch event.Name { - case "on_apps_uninstall_app_success": - printUninstallApp(ctx, clients, appID, teamName) - default: - // Ignore the event - } - }, - ) + return env, nil } func confirmUninstall(ctx context.Context, IO iostreams.IOStreamer, cmd *cobra.Command, selection prompts.SelectedApp) (bool, error) { diff --git a/cmd/auth/auth_test.go b/cmd/auth/auth_test.go index 9f37d6d7..55569b66 100644 --- a/cmd/auth/auth_test.go +++ b/cmd/auth/auth_test.go @@ -19,7 +19,6 @@ import ( "testing" "github.com/slackapi/slack-cli/internal/hooks" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -32,7 +31,7 @@ type listMockObject struct { mock.Mock } -func (m *listMockObject) MockListFunction(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger) ([]types.SlackAuth, error) { +func (m *listMockObject) MockListFunction(ctx context.Context, clients *shared.ClientFactory) ([]types.SlackAuth, error) { args := m.Called() return args.Get(0).([]types.SlackAuth), args.Error(1) } diff --git a/cmd/auth/list.go b/cmd/auth/list.go index 2dbd906a..17f3e0b6 100644 --- a/cmd/auth/list.go +++ b/cmd/auth/list.go @@ -18,7 +18,6 @@ import ( "fmt" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/auth" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" @@ -52,34 +51,15 @@ func NewListCommand(clients *shared.ClientFactory) *cobra.Command { // runListCommand will execute the list command func runListCommand(cmd *cobra.Command, clients *shared.ClientFactory) error { ctx := cmd.Context() - log := newListLogger(cmd, clients.IO) - userAuthList, err := listFunc(ctx, clients, log) + userAuthList, err := listFunc(ctx, clients) if err != nil { return err } + printAuthList(cmd, clients.IO, userAuthList) printAuthListSuccess(cmd, clients.IO, userAuthList) return nil } -// newListLogger creates a logger instance to receive event notifications -func newListLogger(cmd *cobra.Command, IO iostreams.IOStreamer) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_auth_list": - userAuthList := []types.SlackAuth{} - if event.Data["userAuthList"] != nil { - userAuthList = event.Data["userAuthList"].([]types.SlackAuth) - } - printAuthList(cmd, IO, userAuthList) - default: - // Ignore the event - } - }, - ) -} - // printAuthList will display a list of all authorizations available and highlight the default. // The output will be a list formatted as: // diff --git a/cmd/auth/list_test.go b/cmd/auth/list_test.go index 40070c93..35581a21 100644 --- a/cmd/auth/list_test.go +++ b/cmd/auth/list_test.go @@ -18,7 +18,6 @@ import ( "context" "testing" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -32,7 +31,7 @@ type ListPkgMock struct { mock.Mock } -func (m *ListPkgMock) List(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger) ([]types.SlackAuth, error) { +func (m *ListPkgMock) List(ctx context.Context, clients *shared.ClientFactory) ([]types.SlackAuth, error) { m.Called() return []types.SlackAuth{}, nil } diff --git a/cmd/datastore/bulk_delete.go b/cmd/datastore/bulk_delete.go index cffc8b79..fbc21084 100644 --- a/cmd/datastore/bulk_delete.go +++ b/cmd/datastore/bulk_delete.go @@ -21,7 +21,6 @@ import ( "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/goutils" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/datastore" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -88,12 +87,12 @@ func NewBulkDeleteCommand(clients *shared.ClientFactory) *cobra.Command { } // Perform the delete - log := newBulkDeleteLogger(clients, cmd) - event, err := BulkDelete(ctx, clients, log, query) + result, err := BulkDelete(ctx, clients, query) if err != nil { return err } - printDatastoreBulkDeleteSuccess(cmd, event) + _ = printBulkDeleteResult(clients, cmd, result) + printDatastoreBulkDeleteSuccess(cmd) return nil }, } @@ -118,26 +117,6 @@ func preRunBulkDeleteCommandFunc(ctx context.Context, clients *shared.ClientFact return cmdutil.IsSlackHostedProject(ctx, clients) } -func newBulkDeleteLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_bulk_delete_result": - deleteResult := types.AppDatastoreBulkDeleteResult{} - if event.Data["bulkDeleteResult"] != nil { - deleteResult = event.Data["bulkDeleteResult"].(types.AppDatastoreBulkDeleteResult) - } - if cmd != nil { - _ = printBulkDeleteResult(clients, cmd, deleteResult) - } - default: - // Ignore the event - } - }, - ) -} - func printBulkDeleteResult(clients *shared.ClientFactory, cmd *cobra.Command, deleteResult types.AppDatastoreBulkDeleteResult) error { var datastore = deleteResult.Datastore cmd.Printf( @@ -166,7 +145,7 @@ func printBulkDeleteResult(clients *shared.ClientFactory, cmd *cobra.Command, de return nil } -func printDatastoreBulkDeleteSuccess(cmd *cobra.Command, event *logger.LogEvent) { +func printDatastoreBulkDeleteSuccess(cmd *cobra.Command) { commandText := style.Commandf("datastore query ", true) if cmd != nil { cmd.Printf( diff --git a/cmd/datastore/bulk_delete_test.go b/cmd/datastore/bulk_delete_test.go index cdaf0111..90ff9fa0 100644 --- a/cmd/datastore/bulk_delete_test.go +++ b/cmd/datastore/bulk_delete_test.go @@ -22,7 +22,6 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -36,11 +35,9 @@ type BulkDeletePkgMock struct { mock.Mock } -func (m *BulkDeletePkgMock) BulkDelete(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, query types.AppDatastoreBulkDelete) (*logger.LogEvent, error) { - m.Called(ctx, clients, log, query) - log.Data["bulkDeleteResult"] = types.AppDatastoreBulkDeleteResult{} - log.Log("info", "on_bulk_delete_result") - return log.SuccessEvent(), nil +func (m *BulkDeletePkgMock) BulkDelete(ctx context.Context, clients *shared.ClientFactory, query types.AppDatastoreBulkDelete) (types.AppDatastoreBulkDeleteResult, error) { + m.Called(ctx, clients, query) + return types.AppDatastoreBulkDeleteResult{}, nil } func TestBulkDeleteCommandPreRun(t *testing.T) { @@ -185,7 +182,7 @@ func TestBulkDeleteCommand(t *testing.T) { // Create mocked command bulkDeleteMock := new(BulkDeletePkgMock) BulkDelete = bulkDeleteMock.BulkDelete - bulkDeleteMock.On("BulkDelete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + bulkDeleteMock.On("BulkDelete", mock.Anything, mock.Anything, mock.Anything).Return(types.AppDatastoreBulkDeleteResult{}, nil) cmd := NewBulkDeleteCommand(clients) // TODO: could maybe refactor this to the os/fs mocks level to more clearly communicate "fake being in an app directory" @@ -198,7 +195,7 @@ func TestBulkDeleteCommand(t *testing.T) { // Create mocked command err := cmd.ExecuteContext(ctx) if assert.NoError(t, err) { - bulkDeleteMock.AssertCalled(t, "BulkDelete", mock.Anything, mock.Anything, mock.Anything, tc.Query) + bulkDeleteMock.AssertCalled(t, "BulkDelete", mock.Anything, mock.Anything, tc.Query) } // Cleanup when done diff --git a/cmd/datastore/bulk_get.go b/cmd/datastore/bulk_get.go index 9a0fda6c..dd0b56e1 100644 --- a/cmd/datastore/bulk_get.go +++ b/cmd/datastore/bulk_get.go @@ -22,7 +22,6 @@ import ( "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/goutils" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/datastore" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -90,12 +89,12 @@ func NewBulkGetCommand(clients *shared.ClientFactory) *cobra.Command { } // Perform the get - log := newBulkGetLogger(clients, cmd, query) - event, err := BulkGet(ctx, clients, log, query) + result, err := BulkGet(ctx, clients, query) if err != nil { return err } - printDatastoreBulkGetSuccess(cmd, event) + _ = printBulkGetResult(clients, cmd, query, result) + printDatastoreBulkGetSuccess(cmd) return nil }, } @@ -121,27 +120,6 @@ func preRunBulkGetCommandFunc(ctx context.Context, clients *shared.ClientFactory return cmdutil.IsSlackHostedProject(ctx, clients) } -func newBulkGetLogger(clients *shared.ClientFactory, cmd *cobra.Command, request types.AppDatastoreBulkGet) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_bulk_get_result": - getResult := types.AppDatastoreBulkGetResult{} - if event.Data["bulkGetResult"] != nil { - getResult = event.Data["bulkGetResult"].(types.AppDatastoreBulkGetResult) - } - if cmd != nil { - // TODO: this can raise an error on indentation failures, but not sure how to handle that using logger - _ = printBulkGetResult(clients, cmd, request, getResult) - } - default: - // Ignore the event - } - }, - ) -} - func printBulkGetResult(clients *shared.ClientFactory, cmd *cobra.Command, request types.AppDatastoreBulkGet, getResult types.AppDatastoreBulkGetResult) error { var datastore = getResult.Datastore var items = getResult.Items @@ -189,5 +167,5 @@ func printBulkGetResult(clients *shared.ClientFactory, cmd *cobra.Command, reque return nil } -func printDatastoreBulkGetSuccess(cmd *cobra.Command, event *logger.LogEvent) { +func printDatastoreBulkGetSuccess(cmd *cobra.Command) { } diff --git a/cmd/datastore/bulk_get_test.go b/cmd/datastore/bulk_get_test.go index 18cb16b5..e6883493 100644 --- a/cmd/datastore/bulk_get_test.go +++ b/cmd/datastore/bulk_get_test.go @@ -22,7 +22,6 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -36,11 +35,9 @@ type BulkGetPkgMock struct { mock.Mock } -func (m *BulkGetPkgMock) BulkGet(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, query types.AppDatastoreBulkGet) (*logger.LogEvent, error) { - m.Called(ctx, clients, log, query) - log.Data["bulkGetResult"] = types.AppDatastoreBulkGetResult{} - log.Log("info", "on_bulk_get_result") - return log.SuccessEvent(), nil +func (m *BulkGetPkgMock) BulkGet(ctx context.Context, clients *shared.ClientFactory, query types.AppDatastoreBulkGet) (types.AppDatastoreBulkGetResult, error) { + m.Called(ctx, clients, query) + return types.AppDatastoreBulkGetResult{}, nil } func TestBulkGetCommandPreRun(t *testing.T) { @@ -185,7 +182,7 @@ func TestBulkGetCommand(t *testing.T) { // Prepare mocked command bulkGetMock := new(BulkGetPkgMock) BulkGet = bulkGetMock.BulkGet - bulkGetMock.On("BulkGet", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + bulkGetMock.On("BulkGet", mock.Anything, mock.Anything, mock.Anything).Return(types.AppDatastoreBulkGetResult{}, nil) cmd := NewBulkGetCommand(clients) // TODO: could maybe refactor this to the os/fs mocks level to more clearly communicate "fake being in an app directory" @@ -198,7 +195,7 @@ func TestBulkGetCommand(t *testing.T) { // Perform test err := cmd.ExecuteContext(ctx) if assert.NoError(t, err) { - bulkGetMock.AssertCalled(t, "BulkGet", mock.Anything, mock.Anything, mock.Anything, tc.Query) + bulkGetMock.AssertCalled(t, "BulkGet", mock.Anything, mock.Anything, tc.Query) } // Cleanup when done diff --git a/cmd/datastore/bulk_put.go b/cmd/datastore/bulk_put.go index af984f9a..ff3fa93e 100644 --- a/cmd/datastore/bulk_put.go +++ b/cmd/datastore/bulk_put.go @@ -26,7 +26,6 @@ import ( "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/goutils" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/datastore" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -117,12 +116,12 @@ func NewBulkPutCommand(clients *shared.ClientFactory) *cobra.Command { } // Perform the put - log := newBulkPutLogger(clients, cmd) - event, err := BulkPut(ctx, clients, log, query) + result, err := BulkPut(ctx, clients, query) if err != nil { return err } - printDatastoreBulkPutSuccess(cmd, event) + _ = printBulkPutResult(clients, cmd, result) + printDatastoreBulkPutSuccess(cmd) return nil }, } @@ -148,27 +147,6 @@ func preRunBulkPutCommandFunc(ctx context.Context, clients *shared.ClientFactory return cmdutil.IsSlackHostedProject(ctx, clients) } -func newBulkPutLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_bulk_put_result": - bulkPutResult := types.AppDatastoreBulkPutResult{} - if event.Data["bulkPutResult"] != nil { - bulkPutResult = event.Data["bulkPutResult"].(types.AppDatastoreBulkPutResult) - } - if cmd != nil { - // TODO: this can raise an error on indentation failures, but not sure how to handle that using logger - _ = printBulkPutResult(clients, cmd, bulkPutResult) - } - default: - // Ignore the event - } - }, - ) -} - func printBulkPutResult(clients *shared.ClientFactory, cmd *cobra.Command, putResult types.AppDatastoreBulkPutResult) error { var datastore = putResult.Datastore cmd.Printf( @@ -196,7 +174,7 @@ func printBulkPutResult(clients *shared.ClientFactory, cmd *cobra.Command, putRe return nil } -func printDatastoreBulkPutSuccess(cmd *cobra.Command, event *logger.LogEvent) { +func printDatastoreBulkPutSuccess(cmd *cobra.Command) { commandText := style.Commandf("datastore query ", true) if cmd != nil { cmd.Printf( diff --git a/cmd/datastore/bulk_put_test.go b/cmd/datastore/bulk_put_test.go index 132af80b..e2e9e38f 100644 --- a/cmd/datastore/bulk_put_test.go +++ b/cmd/datastore/bulk_put_test.go @@ -25,7 +25,6 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/goutils" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -41,11 +40,9 @@ type BulkPutPkgMock struct { mock.Mock } -func (m *BulkPutPkgMock) BulkPut(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, query types.AppDatastoreBulkPut) (*logger.LogEvent, error) { - m.Called(ctx, clients, log, query) - log.Data["bulkPutResult"] = types.AppDatastoreBulkPutResult{} - log.Log("info", "on_bulk_put_result") - return log.SuccessEvent(), nil +func (m *BulkPutPkgMock) BulkPut(ctx context.Context, clients *shared.ClientFactory, query types.AppDatastoreBulkPut) (types.AppDatastoreBulkPutResult, error) { + m.Called(ctx, clients, query) + return types.AppDatastoreBulkPutResult{}, nil } func TestBulkPutCommandPreRun(t *testing.T) { @@ -202,7 +199,7 @@ func TestBulkPutCommand(t *testing.T) { // Prepare mocked command bulkPutMock := new(BulkPutPkgMock) BulkPut = bulkPutMock.BulkPut - bulkPutMock.On("BulkPut", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + bulkPutMock.On("BulkPut", mock.Anything, mock.Anything, mock.Anything).Return(types.AppDatastoreBulkPutResult{}, nil) cmd := NewBulkPutCommand(clients) // TODO: could maybe refactor this to the os/fs mocks level to more clearly communicate "fake being in an app directory" @@ -215,7 +212,7 @@ func TestBulkPutCommand(t *testing.T) { // Perform test err := cmd.ExecuteContext(ctx) if assert.NoError(t, err) { - bulkPutMock.AssertCalled(t, "BulkPut", mock.Anything, mock.Anything, mock.Anything, tc.Query) + bulkPutMock.AssertCalled(t, "BulkPut", mock.Anything, mock.Anything, tc.Query) } // Cleanup when done diff --git a/cmd/datastore/delete.go b/cmd/datastore/delete.go index 7f023b1d..b0ebc41b 100644 --- a/cmd/datastore/delete.go +++ b/cmd/datastore/delete.go @@ -23,7 +23,6 @@ import ( "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/datastore" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -98,12 +97,12 @@ func NewDeleteCommand(clients *shared.ClientFactory) *cobra.Command { } // Perform the delete - log := newDeleteLogger(clients, cmd) - event, err := Delete(ctx, clients, log, query) + result, err := Delete(ctx, clients, query) if err != nil { return err } - printDatastoreDeleteSuccess(cmd, event) + printDeleteResult(clients, cmd, result) + printDatastoreDeleteSuccess(cmd) return nil }, } @@ -128,26 +127,6 @@ func preRunDeleteCommandFunc(ctx context.Context, clients *shared.ClientFactory, return cmdutil.IsSlackHostedProject(ctx, clients) } -func newDeleteLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_delete_result": - deleteResult := types.AppDatastoreDeleteResult{} - if event.Data["deleteResult"] != nil { - deleteResult = event.Data["deleteResult"].(types.AppDatastoreDeleteResult) - } - if cmd != nil { - printDeleteResult(clients, cmd, deleteResult) - } - default: - // Ignore the event - } - }, - ) -} - func printDeleteResult(clients *shared.ClientFactory, cmd *cobra.Command, deleteResult types.AppDatastoreDeleteResult) { var datastore = deleteResult.Datastore var id = deleteResult.ID @@ -162,7 +141,7 @@ func printDeleteResult(clients *shared.ClientFactory, cmd *cobra.Command, delete ) } -func printDatastoreDeleteSuccess(cmd *cobra.Command, event *logger.LogEvent) { +func printDatastoreDeleteSuccess(cmd *cobra.Command) { commandText := style.Commandf("datastore query ", true) if cmd != nil { cmd.Printf( diff --git a/cmd/datastore/delete_test.go b/cmd/datastore/delete_test.go index 44ef7d7a..4b2fdcca 100644 --- a/cmd/datastore/delete_test.go +++ b/cmd/datastore/delete_test.go @@ -23,7 +23,6 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -37,11 +36,9 @@ type DeletePkgMock struct { mock.Mock } -func (m *DeletePkgMock) Delete(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, query types.AppDatastoreDelete) (*logger.LogEvent, error) { - m.Called(ctx, clients, log, query) - log.Data["deleteResult"] = types.AppDatastoreDeleteResult{} - log.Log("info", "on_delete_result") - return log.SuccessEvent(), nil +func (m *DeletePkgMock) Delete(ctx context.Context, clients *shared.ClientFactory, query types.AppDatastoreDelete) (types.AppDatastoreDeleteResult, error) { + m.Called(ctx, clients, query) + return types.AppDatastoreDeleteResult{}, nil } func TestDeleteCommandPreRun(t *testing.T) { @@ -213,7 +210,7 @@ func TestDeleteCommand(t *testing.T) { // Create mocked command deleteMock := new(DeletePkgMock) Delete = deleteMock.Delete - deleteMock.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + deleteMock.On("Delete", mock.Anything, mock.Anything, mock.Anything).Return(types.AppDatastoreDeleteResult{}, nil) cmd := NewDeleteCommand(clients) // TODO: could maybe refactor this to the os/fs mocks level to more clearly communicate "fake being in an app directory" @@ -229,7 +226,7 @@ func TestDeleteCommand(t *testing.T) { // Perform test err := cmd.ExecuteContext(ctx) if assert.NoError(t, err) { - deleteMock.AssertCalled(t, "Delete", mock.Anything, mock.Anything, mock.Anything, tc.Query) + deleteMock.AssertCalled(t, "Delete", mock.Anything, mock.Anything, tc.Query) } // Cleanup when done diff --git a/cmd/datastore/get.go b/cmd/datastore/get.go index 19343691..63f3cc29 100644 --- a/cmd/datastore/get.go +++ b/cmd/datastore/get.go @@ -24,7 +24,6 @@ import ( "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/datastore" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -100,12 +99,12 @@ func NewGetCommand(clients *shared.ClientFactory) *cobra.Command { } // Perform the get - log := newGetLogger(clients, cmd) - event, err := Get(ctx, clients, log, query) + result, err := Get(ctx, clients, query) if err != nil { return err } - printDatastoreGetSuccess(cmd, event) + _ = printGetResult(clients, cmd, result) + printDatastoreGetSuccess(cmd) return nil }, } @@ -131,27 +130,6 @@ func preRunGetCommandFunc(ctx context.Context, clients *shared.ClientFactory, cm return cmdutil.IsSlackHostedProject(ctx, clients) } -func newGetLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_get_result": - getResult := types.AppDatastoreGetResult{} - if event.Data["getResult"] != nil { - getResult = event.Data["getResult"].(types.AppDatastoreGetResult) - } - if cmd != nil { - // TODO: this can raise an error on indentation failures, but not sure how to handle that using logger - _ = printGetResult(clients, cmd, getResult) - } - default: - // Ignore the event - } - }, - ) -} - func printGetResult(clients *shared.ClientFactory, cmd *cobra.Command, getResult types.AppDatastoreGetResult) error { var datastore = getResult.Datastore var item = getResult.Item @@ -182,7 +160,7 @@ func printGetResult(clients *shared.ClientFactory, cmd *cobra.Command, getResult return nil } -func printDatastoreGetSuccess(cmd *cobra.Command, event *logger.LogEvent) { +func printDatastoreGetSuccess(cmd *cobra.Command) { if outputFlag == "text" { commandText := style.Commandf("datastore get ", true) if cmd != nil { diff --git a/cmd/datastore/get_test.go b/cmd/datastore/get_test.go index 38e6e599..5fb039ab 100644 --- a/cmd/datastore/get_test.go +++ b/cmd/datastore/get_test.go @@ -23,7 +23,6 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -37,11 +36,9 @@ type GetPkgMock struct { mock.Mock } -func (m *GetPkgMock) Get(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, query types.AppDatastoreGet) (*logger.LogEvent, error) { - m.Called(ctx, clients, log, query) - log.Data["getResult"] = types.AppDatastoreGetResult{} - log.Log("info", "on_get_result") - return log.SuccessEvent(), nil +func (m *GetPkgMock) Get(ctx context.Context, clients *shared.ClientFactory, query types.AppDatastoreGet) (types.AppDatastoreGetResult, error) { + m.Called(ctx, clients, query) + return types.AppDatastoreGetResult{}, nil } func TestGetCommandPreRun(t *testing.T) { @@ -212,7 +209,7 @@ func TestGetCommand(t *testing.T) { // Prepare mocked command getMock := new(GetPkgMock) Get = getMock.Get - getMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + getMock.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(types.AppDatastoreGetResult{}, nil) cmd := NewGetCommand(clients) // TODO: could maybe refactor this to the os/fs mocks level to more clearly communicate "fake being in an app directory" @@ -228,7 +225,7 @@ func TestGetCommand(t *testing.T) { // Perform test err := cmd.ExecuteContext(ctx) if assert.NoError(t, err) { - getMock.AssertCalled(t, "Get", mock.Anything, mock.Anything, mock.Anything, tc.Query) + getMock.AssertCalled(t, "Get", mock.Anything, mock.Anything, tc.Query) } // Cleanup when done diff --git a/cmd/datastore/put.go b/cmd/datastore/put.go index 7128fae4..8f2ffa4f 100644 --- a/cmd/datastore/put.go +++ b/cmd/datastore/put.go @@ -24,7 +24,6 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/datastore" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -99,12 +98,12 @@ func NewPutCommand(clients *shared.ClientFactory) *cobra.Command { } // Perform the put - log := newPutLogger(clients, cmd) - event, err := Put(ctx, clients, log, query) + result, err := Put(ctx, clients, query) if err != nil { return err } - printDatastorePutSuccess(cmd, event) + _ = printPutResult(clients, cmd, result) + printDatastorePutSuccess(cmd) return nil }, } @@ -129,27 +128,6 @@ func preRunPutCommandFunc(ctx context.Context, clients *shared.ClientFactory, cm return cmdutil.IsSlackHostedProject(ctx, clients) } -func newPutLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_put_result": - putResult := types.AppDatastorePutResult{} - if event.Data["putResult"] != nil { - putResult = event.Data["putResult"].(types.AppDatastorePutResult) - } - if cmd != nil { - // TODO: this can raise an error on indentation failures, but not sure how to handle that using logger - _ = printPutResult(clients, cmd, putResult) - } - default: - // Ignore the event - } - }, - ) -} - func printPutResult(clients *shared.ClientFactory, cmd *cobra.Command, putResult types.AppDatastorePutResult) error { var datastore = putResult.Datastore var item = putResult.Item @@ -169,7 +147,7 @@ func printPutResult(clients *shared.ClientFactory, cmd *cobra.Command, putResult return nil } -func printDatastorePutSuccess(cmd *cobra.Command, event *logger.LogEvent) { +func printDatastorePutSuccess(cmd *cobra.Command) { commandText := style.Commandf("datastore query ", true) if cmd != nil { cmd.Printf( diff --git a/cmd/datastore/put_test.go b/cmd/datastore/put_test.go index 18d44830..e2a3e477 100644 --- a/cmd/datastore/put_test.go +++ b/cmd/datastore/put_test.go @@ -23,7 +23,6 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -37,11 +36,9 @@ type PutPkgMock struct { mock.Mock } -func (m *PutPkgMock) Put(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, query types.AppDatastorePut) (*logger.LogEvent, error) { - m.Called(ctx, clients, log, query) - log.Data["putResult"] = types.AppDatastorePutResult{} - log.Log("info", "on_put_result") - return log.SuccessEvent(), nil +func (m *PutPkgMock) Put(ctx context.Context, clients *shared.ClientFactory, query types.AppDatastorePut) (types.AppDatastorePutResult, error) { + m.Called(ctx, clients, query) + return types.AppDatastorePutResult{}, nil } func TestPutCommandPreRun(t *testing.T) { @@ -231,7 +228,7 @@ func TestPutCommand(t *testing.T) { // Prepare mocked command putMock := new(PutPkgMock) Put = putMock.Put - putMock.On("Put", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + putMock.On("Put", mock.Anything, mock.Anything, mock.Anything).Return(types.AppDatastorePutResult{}, nil) cmd := NewPutCommand(clients) // TODO: could maybe refactor this to the os/fs mocks level to more clearly communicate "fake being in an app directory" @@ -247,7 +244,7 @@ func TestPutCommand(t *testing.T) { // Perform test err := cmd.ExecuteContext(ctx) if assert.NoError(t, err) { - putMock.AssertCalled(t, "Put", mock.Anything, mock.Anything, mock.Anything, tc.Query) + putMock.AssertCalled(t, "Put", mock.Anything, mock.Anything, tc.Query) } // Cleanup when done diff --git a/cmd/datastore/query.go b/cmd/datastore/query.go index 405040cd..17e885d8 100644 --- a/cmd/datastore/query.go +++ b/cmd/datastore/query.go @@ -27,7 +27,6 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/datastore" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -126,12 +125,12 @@ func NewQueryCommand(clients *shared.ClientFactory) *cobra.Command { } // Perform the query - log := newQueryLogger(clients, cmd) - event, err := Query(ctx, clients, log, query) + result, err := Query(ctx, clients, query) if err != nil { return err } - printDatastoreQuerySuccess(cmd, event) + _ = printQueryResult(clients, cmd, result) + printDatastoreQuerySuccess(cmd) return nil }, } @@ -168,27 +167,6 @@ func preRunQueryCommandFunc(ctx context.Context, clients *shared.ClientFactory, return cmdutil.IsSlackHostedProject(ctx, clients) } -func newQueryLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_query_result": - queryResult := types.AppDatastoreQueryResult{} - if event.Data["queryResult"] != nil { - queryResult = event.Data["queryResult"].(types.AppDatastoreQueryResult) - } - if cmd != nil { - // TODO: this can raise an error on indentation failures, but not sure how to handle that using logger - _ = printQueryResult(clients, cmd, queryResult) - } - default: - // Ignore the event - } - }, - ) -} - func printQueryResult(clients *shared.ClientFactory, cmd *cobra.Command, queryResult types.AppDatastoreQueryResult) error { var datastore = queryResult.Datastore switch outputFlag { @@ -223,7 +201,7 @@ func printQueryResult(clients *shared.ClientFactory, cmd *cobra.Command, queryRe return nil } -func printDatastoreQuerySuccess(cmd *cobra.Command, event *logger.LogEvent) { +func printDatastoreQuerySuccess(cmd *cobra.Command) { if outputFlag == "text" { commandText := style.Commandf("datastore put ", true) if cmd != nil { diff --git a/cmd/datastore/query_test.go b/cmd/datastore/query_test.go index 3ab83110..0f704327 100644 --- a/cmd/datastore/query_test.go +++ b/cmd/datastore/query_test.go @@ -24,7 +24,6 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -39,11 +38,9 @@ type QueryDatastorePkgMock struct { mock.Mock } -func (m *QueryDatastorePkgMock) Query(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, query types.AppDatastoreQuery) (*logger.LogEvent, error) { - m.Called(ctx, clients, log, query) - log.Data["queryResult"] = types.AppDatastoreQueryResult{} - log.Log("info", "on_query_result") - return log.SuccessEvent(), nil +func (m *QueryDatastorePkgMock) Query(ctx context.Context, clients *shared.ClientFactory, query types.AppDatastoreQuery) (types.AppDatastoreQueryResult, error) { + m.Called(ctx, clients, query) + return types.AppDatastoreQueryResult{}, nil } func TestQueryCommandPreRun(t *testing.T) { @@ -328,7 +325,7 @@ func TestQueryCommand(t *testing.T) { // Prepare mocked command queryMock := new(QueryDatastorePkgMock) Query = queryMock.Query - queryMock.On("Query", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + queryMock.On("Query", mock.Anything, mock.Anything, mock.Anything).Return(types.AppDatastoreQueryResult{}, nil) cmd := NewQueryCommand(clients) // TODO: could maybe refactor this to the os/fs mocks level to more clearly communicate "fake being in an app directory" @@ -345,7 +342,7 @@ func TestQueryCommand(t *testing.T) { // Perform test err := cmd.ExecuteContext(ctx) if assert.NoError(t, err) { - queryMock.AssertCalled(t, "Query", mock.Anything, mock.Anything, mock.Anything, tc.Query) + queryMock.AssertCalled(t, "Query", mock.Anything, mock.Anything, tc.Query) } // Cleanup when done diff --git a/cmd/datastore/update.go b/cmd/datastore/update.go index a691fcd4..5bfba6b8 100644 --- a/cmd/datastore/update.go +++ b/cmd/datastore/update.go @@ -24,7 +24,6 @@ import ( "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/datastore" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -99,12 +98,12 @@ func NewUpdateCommand(clients *shared.ClientFactory) *cobra.Command { } // Perform the update - log := newUpdateLogger(clients, cmd) - event, err := Update(ctx, clients, log, query) + result, err := Update(ctx, clients, query) if err != nil { return err } - printDatastoreUpdateSuccess(cmd, event) + _ = printUpdateResult(clients, cmd, result) + printDatastoreUpdateSuccess(cmd) return nil }, } @@ -129,27 +128,6 @@ func preRunUpdateCommandFunc(ctx context.Context, clients *shared.ClientFactory, return cmdutil.IsSlackHostedProject(ctx, clients) } -func newUpdateLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_update_result": - updateResult := types.AppDatastoreUpdateResult{} - if event.Data["updateResult"] != nil { - updateResult = event.Data["updateResult"].(types.AppDatastoreUpdateResult) - } - if cmd != nil { - // TODO: this can raise an error on indentation failures, but not sure how to handle that using logger - _ = printUpdateResult(clients, cmd, updateResult) - } - default: - // Ignore the event - } - }, - ) -} - func printUpdateResult(clients *shared.ClientFactory, cmd *cobra.Command, updateResult types.AppDatastoreUpdateResult) error { var datastore = updateResult.Datastore var item = updateResult.Item @@ -169,7 +147,7 @@ func printUpdateResult(clients *shared.ClientFactory, cmd *cobra.Command, update return nil } -func printDatastoreUpdateSuccess(cmd *cobra.Command, event *logger.LogEvent) { +func printDatastoreUpdateSuccess(cmd *cobra.Command) { commandText := style.Commandf("datastore query ", true) if cmd != nil { cmd.Printf( diff --git a/cmd/datastore/update_test.go b/cmd/datastore/update_test.go index 67e0d729..41131012 100644 --- a/cmd/datastore/update_test.go +++ b/cmd/datastore/update_test.go @@ -23,7 +23,6 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -37,11 +36,9 @@ type UpdatePkgMock struct { mock.Mock } -func (m *UpdatePkgMock) Update(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, query types.AppDatastoreUpdate) (*logger.LogEvent, error) { - m.Called(ctx, clients, log, query) - log.Data["updateResult"] = types.AppDatastoreUpdateResult{} - log.Log("info", "on_update_result") - return log.SuccessEvent(), nil +func (m *UpdatePkgMock) Update(ctx context.Context, clients *shared.ClientFactory, query types.AppDatastoreUpdate) (types.AppDatastoreUpdateResult, error) { + m.Called(ctx, clients, query) + return types.AppDatastoreUpdateResult{}, nil } func TestUpdateCommandPreRun(t *testing.T) { @@ -329,7 +326,7 @@ func TestUpdateCommand(t *testing.T) { // Prepare mocked command updateMock := new(UpdatePkgMock) Update = updateMock.Update - updateMock.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + updateMock.On("Update", mock.Anything, mock.Anything, mock.Anything).Return(types.AppDatastoreUpdateResult{}, nil) cmd := NewUpdateCommand(clients) // TODO: could maybe refactor this to the os/fs mocks level to more clearly communicate "fake being in an app directory" @@ -345,7 +342,7 @@ func TestUpdateCommand(t *testing.T) { // Perform test err := cmd.ExecuteContext(ctx) if assert.NoError(t, err) { - updateMock.AssertCalled(t, "Update", mock.Anything, mock.Anything, mock.Anything, tc.Query) + updateMock.AssertCalled(t, "Update", mock.Anything, mock.Anything, tc.Query) } // Cleanup when done diff --git a/cmd/manifest/validate.go b/cmd/manifest/validate.go index c98ee063..f72d0ec2 100644 --- a/cmd/manifest/validate.go +++ b/cmd/manifest/validate.go @@ -23,7 +23,6 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/manifest" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -78,8 +77,7 @@ func NewValidateCommand(clients *shared.ClientFactory) *cobra.Command { clients.Config.ManifestEnv = app.SetManifestEnvTeamVars(clients.Config.ManifestEnv, selection.App.TeamDomain, selection.App.IsDev) - logger := newValidateLogger(clients, cmd) - log, warn, err := manifestValidateFunc(ctx, clients, logger, selection.App, token) + isValid, warn, err := manifestValidateFunc(ctx, clients, selection.App, token) if err != nil { return err } @@ -87,22 +85,19 @@ func NewValidateCommand(clients *shared.ClientFactory) *cobra.Command { clients.IO.PrintWarning(ctx, "%s", warn.Warning(clients.Config.DebugEnabled, "The following warnings were raised during manifest validation")) return nil } - if log != nil { - var isValid = log.DataToBool("isValid") - if isValid { - cmd.Printf( - "\n%s: %s\n", - style.Bold("App Manifest Validation Result"), - style.Styler().Green("Valid"), - ) - clients.IO.PrintTrace(ctx, slacktrace.ManifestValidateSuccess) - } else { - cmd.Printf( - "\n%s: %s\n", - style.Bold("App Manifest Validation Result"), - style.Styler().Red("InValid"), - ) - } + if isValid { + cmd.Printf( + "\n%s: %s\n", + style.Bold("App Manifest Validation Result"), + style.Styler().Green("Valid"), + ) + clients.IO.PrintTrace(ctx, slacktrace.ManifestValidateSuccess) + } else { + cmd.Printf( + "\n%s: %s\n", + style.Bold("App Manifest Validation Result"), + style.Styler().Red("InValid"), + ) } return nil }, @@ -111,19 +106,6 @@ func NewValidateCommand(clients *shared.ClientFactory) *cobra.Command { return cmd } -// newValidateLogger creates a logger instance to receive event notifications -func newValidateLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - default: - // Ignore the event - } - }, - ) -} - // gatherAuthenticationToken returns some user token and configures authentication // internals for API use func gatherAuthenticationToken(ctx context.Context, clients *shared.ClientFactory) (auth types.SlackAuth, err error) { diff --git a/cmd/manifest/validate_test.go b/cmd/manifest/validate_test.go index 912bdd1e..3cb33f6d 100644 --- a/cmd/manifest/validate_test.go +++ b/cmd/manifest/validate_test.go @@ -21,7 +21,6 @@ import ( "github.com/slackapi/slack-cli/internal/hooks" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" @@ -37,15 +36,16 @@ type ManifestValidatePkgMock struct { mock.Mock } -func (m *ManifestValidatePkgMock) ManifestValidate(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, app types.App, token string) (*logger.LogEvent, slackerror.Warnings, error) { - m.Called(ctx, clients, log, app, token) - return log.SuccessEvent(), nil, nil +func (m *ManifestValidatePkgMock) ManifestValidate(ctx context.Context, clients *shared.ClientFactory, app types.App, token string) (bool, slackerror.Warnings, error) { + m.Called(ctx, clients, app, token) + return true, nil, nil } func TestManifestValidateCommand(t *testing.T) { // Create mocks ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() + clientsMock.AddDefaultMocks() // Create clients that is mocked for testing clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(clients *shared.ClientFactory) { @@ -63,13 +63,13 @@ func TestManifestValidateCommand(t *testing.T) { manifestValidatePkgMock := new(ManifestValidatePkgMock) manifestValidateFunc = manifestValidatePkgMock.ManifestValidate - manifestValidatePkgMock.On("ManifestValidate", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + manifestValidatePkgMock.On("ManifestValidate", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) err := cmd.ExecuteContext(ctx) if err != nil { assert.Fail(t, "cmd.Execute had unexpected error") } - manifestValidatePkgMock.AssertCalled(t, "ManifestValidate", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) + manifestValidatePkgMock.AssertCalled(t, "ManifestValidate", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } func TestManifestValidateCommand_HandleMissingAppInstallError_ZeroUserAuth(t *testing.T) { @@ -138,7 +138,7 @@ func TestManifestValidateCommand_HandleMissingAppInstallError_OneUserAuth(t *tes // Mock the manifest validate package manifestValidatePkgMock := new(ManifestValidatePkgMock) manifestValidateFunc = manifestValidatePkgMock.ManifestValidate - manifestValidatePkgMock.On("ManifestValidate", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + manifestValidatePkgMock.On("ManifestValidate", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) // Should execute without error err := cmd.ExecuteContext(ctx) @@ -200,7 +200,7 @@ func TestManifestValidateCommand_HandleMissingAppInstallError_MoreThanOneUserAut // Mock the manifest validate package manifestValidatePkgMock := new(ManifestValidatePkgMock) manifestValidateFunc = manifestValidatePkgMock.ManifestValidate - manifestValidatePkgMock.On("ManifestValidate", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + manifestValidatePkgMock.On("ManifestValidate", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) // Should execute without error err := cmd.ExecuteContext(ctx) diff --git a/cmd/platform/activity.go b/cmd/platform/activity.go index 72aa908a..52b7334a 100644 --- a/cmd/platform/activity.go +++ b/cmd/platform/activity.go @@ -19,7 +19,6 @@ import ( "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/platform" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -130,23 +129,9 @@ func runActivityCommand(clients *shared.ClientFactory, cmd *cobra.Command, args TraceID: traceID, } - log := newActivityLogger(cmd) - if err := activityFunc(ctx, clients, log, activityArgs); err != nil { + if err := activityFunc(ctx, clients, activityArgs); err != nil { return err } return nil } - -// newActivityLogger creates a logger instance to receive event notifications -func newActivityLogger(cmd *cobra.Command) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - default: - // Ignore the event - } - }, - ) -} diff --git a/cmd/platform/activity_test.go b/cmd/platform/activity_test.go index 59e1932c..ab699acd 100644 --- a/cmd/platform/activity_test.go +++ b/cmd/platform/activity_test.go @@ -19,7 +19,6 @@ import ( "testing" "github.com/slackapi/slack-cli/internal/hooks" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" @@ -38,7 +37,6 @@ type ActivityPkgMock struct { func (m *ActivityPkgMock) Activity( ctx context.Context, clients *shared.ClientFactory, - log *logger.Logger, args types.ActivityArgs, ) error { m.Called() diff --git a/internal/pkg/apps/delete.go b/internal/pkg/apps/delete.go index dabd9fa2..e3485b64 100644 --- a/internal/pkg/apps/delete.go +++ b/internal/pkg/apps/delete.go @@ -20,14 +20,13 @@ 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/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" ) // Delete will delete the app for this teamDomain both remotely (API) and locally (project) -func Delete(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, teamDomain string, app types.App, auth types.SlackAuth) (types.App, error) { +func Delete(ctx context.Context, clients *shared.ClientFactory, teamDomain string, app types.App, auth types.SlackAuth) (types.App, string, error) { span, _ := opentracing.StartSpanFromContext(ctx, "pkg.apps.delete") defer span.Finish() @@ -37,36 +36,29 @@ func Delete(ctx context.Context, clients *shared.ClientFactory, log *logger.Logg // Get Team Name ctx, authSession, err := getAuthSession(ctx, clients, auth) if err != nil { - return types.App{}, slackerror.Wrap(err, slackerror.ErrAppRemove) + return types.App{}, "", slackerror.Wrap(err, slackerror.ErrAppRemove) } - // Is this needed anymore? + var teamName string if authSession.TeamName != nil { - log.Data["teamName"] = *authSession.TeamName + teamName = *authSession.TeamName } - // Emit starting to remove app (requires teamName) - log.Info("on_apps_delete_init") - if app.AppID == "" { err = slackerror.New(slackerror.ErrAppNotFound).WithMessage("App not found for team '%s'", teamDomain) - return types.App{}, slackerror.Wrap(err, slackerror.ErrAppRemove) + return types.App{}, "", slackerror.Wrap(err, slackerror.ErrAppRemove) } - log.Info("on_apps_delete_app_init") - // Delete app remotely via Slack API err = clients.API().DeleteApp(ctx, config.GetContextToken(ctx), app.AppID) if err != nil { - return app, err + return app, teamName, err } - log.Info("on_apps_delete_app_success") // Remove the saved app from project files - log.Info("on_apps_remove_project") removedApp, err := clients.AppClient().Remove(ctx, app) if err != nil { - return types.App{}, err + return types.App{}, teamName, err } if removedApp.IsNew() { clients.IO.PrintDebug( @@ -76,7 +68,7 @@ func Delete(ctx context.Context, clients *shared.ClientFactory, log *logger.Logg app.TeamID, ) } - return app, nil + return app, teamName, nil } // getAuthSession return the api.AuthSession for the current auth diff --git a/internal/pkg/apps/delete_test.go b/internal/pkg/apps/delete_test.go index ca16125b..117068c9 100644 --- a/internal/pkg/apps/delete_test.go +++ b/internal/pkg/apps/delete_test.go @@ -18,7 +18,6 @@ import ( "testing" "github.com/slackapi/slack-cli/internal/api" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -107,7 +106,7 @@ func TestAppsDelete(t *testing.T) { } } - app, err := Delete(ctx, clients, logger.New(nil), tc.app.TeamDomain, tc.app, tc.auth) + app, _, err := Delete(ctx, clients, tc.app.TeamDomain, tc.app, tc.auth) require.NoError(t, err) assert.Equal(t, tc.app, app) clientsMock.API.AssertCalled( diff --git a/internal/pkg/apps/uninstall.go b/internal/pkg/apps/uninstall.go index e10c0b93..aeed7c2d 100644 --- a/internal/pkg/apps/uninstall.go +++ b/internal/pkg/apps/uninstall.go @@ -19,7 +19,6 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" @@ -27,7 +26,7 @@ import ( // Uninstall will uninstall the app that belongs to the teamDomain from the backend. // It will not modify the local project files (apps.json). -func Uninstall(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, teamDomain string, app types.App, auth types.SlackAuth) (types.App, error) { +func Uninstall(ctx context.Context, clients *shared.ClientFactory, teamDomain string, app types.App, auth types.SlackAuth) (types.App, string, error) { span, _ := opentracing.StartSpanFromContext(ctx, "pkg.apps.uninstall") defer span.Finish() @@ -37,32 +36,27 @@ func Uninstall(ctx context.Context, clients *shared.ClientFactory, log *logger.L // Get Team Name ctx, authSession, err := getAuthSession(ctx, clients, auth) if err != nil { - return types.App{}, slackerror.Wrap(err, slackerror.ErrAppRemove) + return types.App{}, "", slackerror.Wrap(err, slackerror.ErrAppRemove) } + + var teamName string if authSession.TeamName != nil { - log.Data["teamName"] = *authSession.TeamName + teamName = *authSession.TeamName } - // Emit starting to remove app (requires teamName) - log.Info("on_apps_uninstall_init") - if app.AppID == "" { err = slackerror.New("app for team " + teamDomain + " not found") - return types.App{}, slackerror.Wrap(err, slackerror.ErrAppRemove) + return types.App{}, "", slackerror.Wrap(err, slackerror.ErrAppRemove) } // Get token token := config.GetContextToken(ctx) // Uninstall the app - log.Info("on_apps_uninstall_app_init") err = clients.API().UninstallApp(ctx, token, app.AppID, app.TeamID) if err != nil { - return app, err + return app, teamName, err } - log.Info("on_apps_uninstall_app_success") - - // Not modifying apps.json / apps.dev.json - return app, nil + return app, teamName, nil } diff --git a/internal/pkg/auth/list.go b/internal/pkg/auth/list.go index e80e1419..4002bddd 100644 --- a/internal/pkg/auth/list.go +++ b/internal/pkg/auth/list.go @@ -19,14 +19,13 @@ import ( "sort" "github.com/opentracing/opentracing-go" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" ) // List returns a list of the authenticated Slack accounts. -func List(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger) ([]types.SlackAuth, error) { +func List(ctx context.Context, clients *shared.ClientFactory) ([]types.SlackAuth, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.auth.list") defer span.Finish() @@ -40,10 +39,5 @@ func List(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger return auths[i].TeamDomain < auths[j].TeamDomain }) - // Notify listeners - log.Data = logger.LogData{} - log.Data["userAuthList"] = auths - log.Log("info", "on_auth_list") - return auths, nil } diff --git a/internal/pkg/auth/list_test.go b/internal/pkg/auth/list_test.go index f82a3daa..edeac6ad 100644 --- a/internal/pkg/auth/list_test.go +++ b/internal/pkg/auth/list_test.go @@ -20,7 +20,6 @@ import ( "time" "github.com/slackapi/slack-cli/internal/hooks" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -52,7 +51,7 @@ func TestAuthList(t *testing.T) { clientsMock.Auth.On("Auths", mock.Anything).Return(mockAuths, errors.New("There was an error")) t.Run("Handles error getting auth slice", func(t *testing.T) { - _, err := List(ctx, clients, &logger.Logger{}) + _, err := List(ctx, clients) require.Error(t, err) }) } @@ -94,7 +93,7 @@ func TestAuthList_SortedAuths(t *testing.T) { clientsMock.Auth.On("Auths", mock.Anything).Return(mockAuths, nil) - _, err := List(ctx, clients, &logger.Logger{}) + _, err := List(ctx, clients) require.NoError(t, err) // require.Equal(t, expectSortedAuths[0], authB, "should sort alphabetically as first elem") diff --git a/internal/pkg/datastore/bulk_delete.go b/internal/pkg/datastore/bulk_delete.go index 8015bc87..c6236b0f 100644 --- a/internal/pkg/datastore/bulk_delete.go +++ b/internal/pkg/datastore/bulk_delete.go @@ -19,12 +19,11 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" ) -func BulkDelete(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, request types.AppDatastoreBulkDelete) (*logger.LogEvent, error) { +func BulkDelete(ctx context.Context, clients *shared.ClientFactory, request types.AppDatastoreBulkDelete) (types.AppDatastoreBulkDeleteResult, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.datastore.bulkDelete") defer span.Finish() @@ -33,12 +32,8 @@ func BulkDelete(ctx context.Context, clients *shared.ClientFactory, log *logger. deleteResult, err := clients.API().AppsDatastoreBulkDelete(ctx, token, request) if err != nil { - return nil, err + return types.AppDatastoreBulkDeleteResult{}, err } - // Notify listeners - log.Data["bulkDeleteResult"] = deleteResult - log.Log("info", "on_bulk_delete_result") - - return log.SuccessEvent(), nil + return deleteResult, nil } diff --git a/internal/pkg/datastore/bulk_delete_test.go b/internal/pkg/datastore/bulk_delete_test.go index 1936bd7e..815956ba 100644 --- a/internal/pkg/datastore/bulk_delete_test.go +++ b/internal/pkg/datastore/bulk_delete_test.go @@ -17,7 +17,6 @@ package datastore import ( "testing" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -59,16 +58,13 @@ func TestDatastoreBulkDeleteArguments(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() - log := logger.Logger{ - Data: map[string]interface{}{}, - } clientsMock.API.On("AppsDatastoreBulkDelete", mock.Anything, mock.Anything, tc.Query). Return(tc.Results, nil) client := shared.NewClientFactory(clientsMock.MockClientFactory()) - event, err := BulkDelete(ctx, client, &log, tc.Query) + result, err := BulkDelete(ctx, client, tc.Query) if assert.NoError(t, err) { - assert.Equal(t, tc.Results, event.Data["bulkDeleteResult"]) + assert.Equal(t, tc.Results, result) } }) } diff --git a/internal/pkg/datastore/bulk_get.go b/internal/pkg/datastore/bulk_get.go index 2aeff35c..7058d983 100644 --- a/internal/pkg/datastore/bulk_get.go +++ b/internal/pkg/datastore/bulk_get.go @@ -19,12 +19,11 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" ) -func BulkGet(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, request types.AppDatastoreBulkGet) (*logger.LogEvent, error) { +func BulkGet(ctx context.Context, clients *shared.ClientFactory, request types.AppDatastoreBulkGet) (types.AppDatastoreBulkGetResult, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.datastore.bulkGet") defer span.Finish() @@ -33,12 +32,8 @@ func BulkGet(ctx context.Context, clients *shared.ClientFactory, log *logger.Log getResult, err := clients.API().AppsDatastoreBulkGet(ctx, token, request) if err != nil { - return nil, err + return types.AppDatastoreBulkGetResult{}, err } - // Notify listeners - log.Data["bulkGetResult"] = getResult - log.Log("info", "on_bulk_get_result") - - return log.SuccessEvent(), nil + return getResult, nil } diff --git a/internal/pkg/datastore/bulk_get_test.go b/internal/pkg/datastore/bulk_get_test.go index 3b1fa4dc..486cba44 100644 --- a/internal/pkg/datastore/bulk_get_test.go +++ b/internal/pkg/datastore/bulk_get_test.go @@ -17,7 +17,6 @@ package datastore import ( "testing" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -60,16 +59,13 @@ func TestDatastoreBulkGetArguments(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() - log := logger.Logger{ - Data: map[string]interface{}{}, - } clientsMock.API.On("AppsDatastoreBulkGet", mock.Anything, mock.Anything, tc.Query). Return(tc.Results, nil) client := shared.NewClientFactory(clientsMock.MockClientFactory()) - event, err := BulkGet(ctx, client, &log, tc.Query) + result, err := BulkGet(ctx, client, tc.Query) if assert.NoError(t, err) { - assert.Equal(t, tc.Results, event.Data["bulkGetResult"]) + assert.Equal(t, tc.Results, result) } }) } diff --git a/internal/pkg/datastore/bulk_put.go b/internal/pkg/datastore/bulk_put.go index 5e23ed58..c145125e 100644 --- a/internal/pkg/datastore/bulk_put.go +++ b/internal/pkg/datastore/bulk_put.go @@ -19,12 +19,11 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" ) -func BulkPut(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, request types.AppDatastoreBulkPut) (*logger.LogEvent, error) { +func BulkPut(ctx context.Context, clients *shared.ClientFactory, request types.AppDatastoreBulkPut) (types.AppDatastoreBulkPutResult, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.datastore.bulkPut") defer span.Finish() @@ -33,12 +32,8 @@ func BulkPut(ctx context.Context, clients *shared.ClientFactory, log *logger.Log bulkPutResult, err := clients.API().AppsDatastoreBulkPut(ctx, token, request) if err != nil { - return nil, err + return types.AppDatastoreBulkPutResult{}, err } - // Notify listeners - log.Data["bulkPutResult"] = bulkPutResult - log.Log("info", "on_bulk_put_result") - - return log.SuccessEvent(), nil + return bulkPutResult, nil } diff --git a/internal/pkg/datastore/bulk_put_test.go b/internal/pkg/datastore/bulk_put_test.go index ac9c6211..9253eec5 100644 --- a/internal/pkg/datastore/bulk_put_test.go +++ b/internal/pkg/datastore/bulk_put_test.go @@ -17,7 +17,6 @@ package datastore import ( "testing" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -67,16 +66,13 @@ func TestDatastoreBulkPutArguments(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() - log := logger.Logger{ - Data: map[string]interface{}{}, - } clientsMock.API.On("AppsDatastoreBulkPut", mock.Anything, mock.Anything, tc.Query). Return(tc.Results, nil) client := shared.NewClientFactory(clientsMock.MockClientFactory()) - event, err := BulkPut(ctx, client, &log, tc.Query) + result, err := BulkPut(ctx, client, tc.Query) if assert.NoError(t, err) { - assert.Equal(t, tc.Results, event.Data["bulkPutResult"]) + assert.Equal(t, tc.Results, result) } }) } diff --git a/internal/pkg/datastore/delete.go b/internal/pkg/datastore/delete.go index e41fb4f0..47a0ccc6 100644 --- a/internal/pkg/datastore/delete.go +++ b/internal/pkg/datastore/delete.go @@ -19,12 +19,11 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" ) -func Delete(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, request types.AppDatastoreDelete) (*logger.LogEvent, error) { +func Delete(ctx context.Context, clients *shared.ClientFactory, request types.AppDatastoreDelete) (types.AppDatastoreDeleteResult, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.datastore.delete") defer span.Finish() @@ -33,12 +32,8 @@ func Delete(ctx context.Context, clients *shared.ClientFactory, log *logger.Logg deleteResult, err := clients.API().AppsDatastoreDelete(ctx, token, request) if err != nil { - return nil, err + return types.AppDatastoreDeleteResult{}, err } - // Notify listeners - log.Data["deleteResult"] = deleteResult - log.Log("info", "on_delete_result") - - return log.SuccessEvent(), nil + return deleteResult, nil } diff --git a/internal/pkg/datastore/delete_test.go b/internal/pkg/datastore/delete_test.go index 72548199..d7885c8b 100644 --- a/internal/pkg/datastore/delete_test.go +++ b/internal/pkg/datastore/delete_test.go @@ -17,7 +17,6 @@ package datastore import ( "testing" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -48,16 +47,13 @@ func TestDatastoreDeleteArguments(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() - log := logger.Logger{ - Data: map[string]interface{}{}, - } clientsMock.API.On("AppsDatastoreDelete", mock.Anything, mock.Anything, tc.Query). Return(tc.Results, nil) client := shared.NewClientFactory(clientsMock.MockClientFactory()) - event, err := Delete(ctx, client, &log, tc.Query) + result, err := Delete(ctx, client, tc.Query) if assert.NoError(t, err) { - assert.Equal(t, tc.Results, event.Data["deleteResult"]) + assert.Equal(t, tc.Results, result) } }) } diff --git a/internal/pkg/datastore/get.go b/internal/pkg/datastore/get.go index 97781ea8..ba7f6e68 100644 --- a/internal/pkg/datastore/get.go +++ b/internal/pkg/datastore/get.go @@ -19,12 +19,11 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" ) -func Get(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, request types.AppDatastoreGet) (*logger.LogEvent, error) { +func Get(ctx context.Context, clients *shared.ClientFactory, request types.AppDatastoreGet) (types.AppDatastoreGetResult, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.datastore.get") defer span.Finish() @@ -33,12 +32,8 @@ func Get(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, getResult, err := clients.API().AppsDatastoreGet(ctx, token, request) if err != nil { - return nil, err + return types.AppDatastoreGetResult{}, err } - // Notify listeners - log.Data["getResult"] = getResult - log.Log("info", "on_get_result") - - return log.SuccessEvent(), nil + return getResult, nil } diff --git a/internal/pkg/datastore/get_test.go b/internal/pkg/datastore/get_test.go index d5134de4..bfaa3f40 100644 --- a/internal/pkg/datastore/get_test.go +++ b/internal/pkg/datastore/get_test.go @@ -17,7 +17,6 @@ package datastore import ( "testing" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -48,16 +47,13 @@ func TestDatastoreGetArguments(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() - log := logger.Logger{ - Data: map[string]interface{}{}, - } clientsMock.API.On("AppsDatastoreGet", mock.Anything, mock.Anything, tc.Query). Return(tc.Results, nil) client := shared.NewClientFactory(clientsMock.MockClientFactory()) - event, err := Get(ctx, client, &log, tc.Query) + result, err := Get(ctx, client, tc.Query) if assert.NoError(t, err) { - assert.Equal(t, tc.Results, event.Data["getResult"]) + assert.Equal(t, tc.Results, result) } }) } diff --git a/internal/pkg/datastore/put.go b/internal/pkg/datastore/put.go index 12ef1e42..8da9b32e 100644 --- a/internal/pkg/datastore/put.go +++ b/internal/pkg/datastore/put.go @@ -19,12 +19,11 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" ) -func Put(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, request types.AppDatastorePut) (*logger.LogEvent, error) { +func Put(ctx context.Context, clients *shared.ClientFactory, request types.AppDatastorePut) (types.AppDatastorePutResult, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.datastore.put") defer span.Finish() @@ -33,12 +32,8 @@ func Put(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, putResult, err := clients.API().AppsDatastorePut(ctx, token, request) if err != nil { - return nil, err + return types.AppDatastorePutResult{}, err } - // Notify listeners - log.Data["putResult"] = putResult - log.Log("info", "on_put_result") - - return log.SuccessEvent(), nil + return putResult, nil } diff --git a/internal/pkg/datastore/put_test.go b/internal/pkg/datastore/put_test.go index 6f34bf2b..d6625fcf 100644 --- a/internal/pkg/datastore/put_test.go +++ b/internal/pkg/datastore/put_test.go @@ -17,7 +17,6 @@ package datastore import ( "testing" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -48,16 +47,13 @@ func TestDatastorePutArguments(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() - log := logger.Logger{ - Data: map[string]interface{}{}, - } clientsMock.API.On("AppsDatastorePut", mock.Anything, mock.Anything, tc.Query). Return(tc.Results, nil) client := shared.NewClientFactory(clientsMock.MockClientFactory()) - event, err := Put(ctx, client, &log, tc.Query) + result, err := Put(ctx, client, tc.Query) if assert.NoError(t, err) { - assert.Equal(t, tc.Results, event.Data["putResult"]) + assert.Equal(t, tc.Results, result) } }) } diff --git a/internal/pkg/datastore/query.go b/internal/pkg/datastore/query.go index 88fa1a4f..22623a75 100644 --- a/internal/pkg/datastore/query.go +++ b/internal/pkg/datastore/query.go @@ -19,12 +19,11 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" ) -func Query(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, request types.AppDatastoreQuery) (*logger.LogEvent, error) { +func Query(ctx context.Context, clients *shared.ClientFactory, request types.AppDatastoreQuery) (types.AppDatastoreQueryResult, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.datastore.query") defer span.Finish() @@ -33,11 +32,8 @@ func Query(ctx context.Context, clients *shared.ClientFactory, log *logger.Logge queryResult, err := clients.API().AppsDatastoreQuery(ctx, token, request) if err != nil { - return nil, err + return types.AppDatastoreQueryResult{}, err } - // Notify listeners - log.Data["queryResult"] = queryResult - log.Log("info", "on_query_result") - return log.SuccessEvent(), nil + return queryResult, nil } diff --git a/internal/pkg/datastore/query_test.go b/internal/pkg/datastore/query_test.go index 02fba8a6..c6106cac 100644 --- a/internal/pkg/datastore/query_test.go +++ b/internal/pkg/datastore/query_test.go @@ -17,7 +17,6 @@ package datastore import ( "testing" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -106,16 +105,13 @@ func TestDatastoreQueryArguments(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() - log := logger.Logger{ - Data: map[string]interface{}{}, - } clientsMock.API.On("AppsDatastoreQuery", mock.Anything, mock.Anything, tc.Query). Return(tc.Results, nil) client := shared.NewClientFactory(clientsMock.MockClientFactory()) - event, err := Query(ctx, client, &log, tc.Query) + result, err := Query(ctx, client, tc.Query) if assert.NoError(t, err) { - assert.Equal(t, tc.Results, event.Data["queryResult"]) + assert.Equal(t, tc.Results, result) } }) } diff --git a/internal/pkg/datastore/update.go b/internal/pkg/datastore/update.go index 6360bfc1..3298c6bd 100644 --- a/internal/pkg/datastore/update.go +++ b/internal/pkg/datastore/update.go @@ -19,12 +19,11 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" ) -func Update(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, request types.AppDatastoreUpdate) (*logger.LogEvent, error) { +func Update(ctx context.Context, clients *shared.ClientFactory, request types.AppDatastoreUpdate) (types.AppDatastoreUpdateResult, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.datastore.update") defer span.Finish() @@ -33,12 +32,8 @@ func Update(ctx context.Context, clients *shared.ClientFactory, log *logger.Logg updateResult, err := clients.API().AppsDatastoreUpdate(ctx, token, request) if err != nil { - return nil, err + return types.AppDatastoreUpdateResult{}, err } - // Notify listeners - log.Data["updateResult"] = updateResult - log.Log("info", "on_update_result") - - return log.SuccessEvent(), nil + return updateResult, nil } diff --git a/internal/pkg/datastore/update_test.go b/internal/pkg/datastore/update_test.go index a5015c19..18a1879b 100644 --- a/internal/pkg/datastore/update_test.go +++ b/internal/pkg/datastore/update_test.go @@ -17,7 +17,6 @@ package datastore import ( "testing" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -48,16 +47,13 @@ func TestDatastoreUpdateArguments(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() - log := logger.Logger{ - Data: map[string]interface{}{}, - } clientsMock.API.On("AppsDatastoreUpdate", mock.Anything, mock.Anything, tc.Query). Return(tc.Results, nil) client := shared.NewClientFactory(clientsMock.MockClientFactory()) - event, err := Update(ctx, client, &log, tc.Query) + result, err := Update(ctx, client, tc.Query) if assert.NoError(t, err) { - assert.Equal(t, tc.Results, event.Data["updateResult"]) + assert.Equal(t, tc.Results, result) } }) } diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index 50d04109..7bcaaa04 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -23,7 +23,6 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" @@ -32,22 +31,22 @@ import ( ) // ManifestValidate validates the manifest from the project "get-manifest" hook -func ManifestValidate(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, app types.App, token string) (*logger.LogEvent, slackerror.Warnings, error) { +func ManifestValidate(ctx context.Context, clients *shared.ClientFactory, app types.App, token string) (bool, slackerror.Warnings, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.ManifestValidate") defer span.Finish() if strings.TrimSpace(token) == "" { - return nil, nil, slackerror.New(slackerror.ErrAuthToken) + return false, nil, slackerror.New(slackerror.ErrAuthToken) } _, err := clients.API().ValidateSession(ctx, token) if err != nil { - return nil, nil, slackerror.New(slackerror.ErrAuthToken).WithRootCause(err) + return false, nil, slackerror.New(slackerror.ErrAuthToken).WithRootCause(err) } slackManifest, err := clients.AppClient().Manifest.GetManifestLocal(ctx, clients.SDKConfig, clients.HookExecutor) if err != nil { - return nil, nil, slackerror.Wrap(err, slackerror.ErrAppManifestGenerate) + return false, nil, slackerror.Wrap(err, slackerror.ErrAppManifestGenerate) } // validate the manifest @@ -58,15 +57,14 @@ func ManifestValidate(ctx context.Context, clients *shared.ClientFactory, log *l } if err := HandleConnectorApprovalRequired(ctx, clients, token, err); err != nil { - return nil, nil, err + return false, nil, err } if err != nil || len(validationResult.Warnings) > 0 { - return nil, validationResult.Warnings, err + return false, validationResult.Warnings, err } - log.Data["isValid"] = true - return log.SuccessEvent(), nil, nil + return true, nil, nil } // HandleConnectorNotInstalled attempts install the certified app associated with any connector_not_installed error diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index e3b20508..e3999d17 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -21,7 +21,6 @@ import ( "github.com/slackapi/slack-cli/internal/api" "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/hooks" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -31,7 +30,7 @@ import ( ) func Test_ManifestValidate_GetManifestLocal_Error(t *testing.T) { - ctx, clients, _, log, appMock, authMock := setupCommonMocks(t) + ctx, clients, _, appMock, authMock := setupCommonMocks(t) // Mock the manifest to return error on get manifestMock := &app.ManifestMockObject{} @@ -39,32 +38,32 @@ func Test_ManifestValidate_GetManifestLocal_Error(t *testing.T) { clients.AppClient().Manifest = manifestMock // Test - logEvent, _, err := ManifestValidate(ctx, clients, log, appMock, authMock.Token) + isValid, _, err := ManifestValidate(ctx, clients, appMock, authMock.Token) assert.Error(t, err) - assert.Nil(t, logEvent) + assert.False(t, isValid) } func Test_ManifestValidate_Success(t *testing.T) { t.Run("should return success when no errors or warnings", func(t *testing.T) { - ctx, clients, clientsMock, log, appMock, authMock := setupCommonMocks(t) + ctx, clients, clientsMock, appMock, authMock := setupCommonMocks(t) // Mock manifest validation api result with no error clientsMock.API.On("ValidateAppManifest", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(api.ValidateAppManifestResult{}, nil) // Test - logEvent, _, err := ManifestValidate(ctx, clients, log, appMock, authMock.Token) + isValid, _, err := ManifestValidate(ctx, clients, appMock, authMock.Token) assert.NoError(t, err) - assert.Equal(t, "success", logEvent.Name) + assert.True(t, isValid) }) } func Test_ManifestValidate_Warnings(t *testing.T) { t.Run("should return warnings", func(t *testing.T) { - ctx, clients, clientsMock, log, appMock, authMock := setupCommonMocks(t) + ctx, clients, clientsMock, appMock, authMock := setupCommonMocks(t) // Mock manifest validation api result with no error clientsMock.API.On("ValidateAppManifest", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(api.ValidateAppManifestResult{ @@ -76,7 +75,7 @@ func Test_ManifestValidate_Warnings(t *testing.T) { }, nil) // Test - _, warnings, err := ManifestValidate(ctx, clients, log, appMock, authMock.Token) + _, warnings, err := ManifestValidate(ctx, clients, appMock, authMock.Token) assert.NoError(t, err) assert.NoError(t, err) @@ -88,7 +87,7 @@ func Test_ManifestValidate_Warnings(t *testing.T) { func Test_ManifestValidate_Error(t *testing.T) { t.Run("should return error when there are errors", func(t *testing.T) { - ctx, clients, clientsMock, log, appMock, authMock := setupCommonMocks(t) + ctx, clients, clientsMock, appMock, authMock := setupCommonMocks(t) // Mock manifest validation api result with an error clientsMock.API.On("ValidateAppManifest", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return( @@ -100,16 +99,16 @@ func Test_ManifestValidate_Error(t *testing.T) { })) // Test - logEvent, _, err := ManifestValidate(ctx, clients, log, appMock, authMock.Token) + isValid, _, err := ManifestValidate(ctx, clients, appMock, authMock.Token) - assert.Nil(t, logEvent) + assert.False(t, isValid) assert.Error(t, err) }) } func Test_ManifestValidate_Error_ErrConnectorNotInstalled(t *testing.T) { t.Run("should try to install connector apps when there are related errors", func(t *testing.T) { - ctx, clients, clientsMock, log, appMock, authMock := setupCommonMocks(t) + ctx, clients, clientsMock, appMock, authMock := setupCommonMocks(t) // Mock manifest validation api result with an error and error details clientsMock.API.On("ValidateAppManifest", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(api.ValidateAppManifestResult{ @@ -133,7 +132,7 @@ func Test_ManifestValidate_Error_ErrConnectorNotInstalled(t *testing.T) { clientsMock.API.On("CertifiedAppInstall", mock.Anything, authMock.Token, mock.Anything).Return(api.CertifiedInstallResult{}, nil) // Test - _, _, err := ManifestValidate(ctx, clients, log, appMock, authMock.Token) + _, _, err := ManifestValidate(ctx, clients, appMock, authMock.Token) // Since we've mocked the ValidateAppManifest call to return an error, we still expect this method to return an error // despite a successful CertifiedAppInstall call. That is realistic given that a manifest can error for other reasons @@ -151,7 +150,7 @@ func Test_HandleConnectorApprovalRequired(t *testing.T) { testReason := "GIVE IT TO ME!" t.Run("should send request to approve connector", func(t *testing.T) { - ctx, clients, clientsMock, _, _, authMock := setupCommonMocks(t) + ctx, clients, clientsMock, _, authMock := setupCommonMocks(t) testErr := slackerror.New("a dummy error").WithDetails(slackerror.ErrorDetails{ slackerror.ErrorDetail{ Code: slackerror.ErrConnectorApprovalRequired, @@ -188,7 +187,7 @@ func Test_HandleConnectorApprovalRequired(t *testing.T) { }) t.Run("should not send request to approve connector if user refuses", func(t *testing.T) { - ctx, clients, clientsMock, _, _, authMock := setupCommonMocks(t) + ctx, clients, clientsMock, _, authMock := setupCommonMocks(t) testErr := slackerror.New("a dummy error").WithDetails(slackerror.ErrorDetails{ slackerror.ErrorDetail{ Code: slackerror.ErrConnectorApprovalRequired, @@ -209,7 +208,7 @@ func Test_HandleConnectorApprovalRequired(t *testing.T) { }) t.Run("should return error if request RequestAppApproval fails", func(t *testing.T) { - ctx, clients, clientsMock, _, _, authMock := setupCommonMocks(t) + ctx, clients, clientsMock, _, authMock := setupCommonMocks(t) testErr := slackerror.New("a dummy error").WithDetails(slackerror.ErrorDetails{ slackerror.ErrorDetail{ Code: slackerror.ErrConnectorApprovalRequired, @@ -232,7 +231,7 @@ func Test_HandleConnectorApprovalRequired(t *testing.T) { } // Setup -func setupCommonMocks(t *testing.T) (ctx context.Context, clients *shared.ClientFactory, clientsMock *shared.ClientsMock, log *logger.Logger, mockApp types.App, mockAuth types.SlackAuth) { +func setupCommonMocks(t *testing.T) (ctx context.Context, clients *shared.ClientFactory, clientsMock *shared.ClientsMock, mockApp types.App, mockAuth types.SlackAuth) { // Create mocks ctx = slackcontext.MockContext(t.Context()) clientsMock = shared.NewClientsMock() @@ -251,10 +250,6 @@ func setupCommonMocks(t *testing.T) (ctx context.Context, clients *shared.Client manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{}, nil) clients.AppClient().Manifest = manifestMock - // Setup logger - log = &logger.Logger{} - log.Data = logger.LogData{} - // Create a dummy app mockApp = types.App{ AppID: "A123", @@ -270,5 +265,5 @@ func setupCommonMocks(t *testing.T) (ctx context.Context, clients *shared.Client TeamDomain: "workspace", } - return ctx, clients, clientsMock, log, mockApp, mockAuth + return ctx, clients, clientsMock, mockApp, mockAuth } diff --git a/internal/pkg/platform/activity.go b/internal/pkg/platform/activity.go index b155c225..1359d1af 100644 --- a/internal/pkg/platform/activity.go +++ b/internal/pkg/platform/activity.go @@ -24,7 +24,6 @@ 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/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" @@ -47,7 +46,6 @@ const ( func Activity( ctx context.Context, clients *shared.ClientFactory, - log *logger.Logger, args types.ActivityArgs, ) error { span, ctx := opentracing.StartSpanFromContext(ctx, "cmd.activity") diff --git a/internal/pkg/platform/activity_test.go b/internal/pkg/platform/activity_test.go index 51f4e25f..2770048c 100644 --- a/internal/pkg/platform/activity_test.go +++ b/internal/pkg/platform/activity_test.go @@ -21,7 +21,6 @@ import ( "github.com/slackapi/slack-cli/internal/api" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -240,7 +239,7 @@ func TestPlatformActivity_StreamingLogs(t *testing.T) { // Setup default mock actions clientsMock.AddDefaultMocks() - err := Activity(ctxMock, clients, &logger.Logger{}, tc.Args) + err := Activity(ctxMock, clients, tc.Args) if tc.ExpectedError != nil { require.Error(t, err) assert.Contains(t, err.Error(), tc.ExpectedError.Error(), err) diff --git a/internal/pkg/platform/localserver.go b/internal/pkg/platform/localserver.go index 5834340c..b5e20a6b 100644 --- a/internal/pkg/platform/localserver.go +++ b/internal/pkg/platform/localserver.go @@ -539,7 +539,7 @@ func (r *LocalServer) WatchActivityLogs(ctx context.Context, minLevel string) er IdleTimeoutM: 60 * 24, } // Next line runs in a ticker loop (based on TailArg above) that will return if the context is cancelled or an error occurs - return Activity(ctx, r.clients, r.log, activityArgs) + return Activity(ctx, r.clients, activityArgs) } // Message describes a web socket incoming message diff --git a/internal/pkg/platform/run.go b/internal/pkg/platform/run.go index 4a9cf36d..f6015cda 100644 --- a/internal/pkg/platform/run.go +++ b/internal/pkg/platform/run.go @@ -207,7 +207,7 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, func deleteAppOnTerminate(ctx context.Context, clients *shared.ClientFactory, auth types.SlackAuth, app types.App, log *logger.Logger) { clients.IO.PrintDebug(ctx, "Removing the local version of this app from the workspace") - _, err := apps.Delete(ctx, clients, log, app.TeamDomain, app, auth) + _, _, err := apps.Delete(ctx, clients, app.TeamDomain, app, auth) if err != nil { log.Data["on_cleanup_app_install_error"] = err.Error() log.Info("on_cleanup_app_install_failed") From ff6c2c4879b3c7a4132de7f11e9300969bf76998 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 5 Mar 2026 16:21:28 -0800 Subject: [PATCH 2/5] refactor: remove logger from project create --- cmd/project/create.go | 84 ++---------------------------- cmd/project/create_test.go | 65 ++++++++++++----------- cmd/project/samples_test.go | 3 +- internal/pkg/create/create.go | 24 +++------ internal/pkg/create/create_test.go | 4 +- 5 files changed, 43 insertions(+), 137 deletions(-) diff --git a/cmd/project/create.go b/cmd/project/create.go index f73dec98..1d97b29b 100644 --- a/cmd/project/create.go +++ b/cmd/project/create.go @@ -23,7 +23,6 @@ import ( "time" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/create" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/slackerror" @@ -45,9 +44,6 @@ var CreateFunc = create.Create var appCreateSpinner *style.Spinner -const copyTemplate = "Copying" -const cloneTemplate = "Cloning" - // promptObject describes the Github app template type promptObject struct { Title string // "Reverse string" @@ -93,9 +89,6 @@ name your app 'agent' (not create an AI Agent), use the --name flag instead.`, func runCreateCommand(clients *shared.ClientFactory, cmd *cobra.Command, args []string) error { ctx := cmd.Context() - // Set up event logger - log := newCreateLogger(clients, cmd) - // Get optional app name passed as an arg and check for category shortcuts appNameArg := "" categoryShortcut := "" @@ -177,88 +170,17 @@ func runCreateCommand(clients *shared.ClientFactory, cmd *cobra.Command, args [] } clients.EventTracker.SetAppTemplate(template.GetTemplatePath()) - appDirPath, err := CreateFunc(ctx, clients, log, createArgs) + appCreateSpinner.Update("Creating app from template", "").Start() + appDirPath, err := CreateFunc(ctx, clients, createArgs) if err != nil { printAppCreateError(clients, cmd, err) return err } + appCreateSpinner.Update(style.Sectionf(style.TextSection{Emoji: "gear", Text: "Created project directory"}), "").Stop() printCreateSuccess(ctx, clients, appDirPath) return nil } -/* -App creation is setting up local project directory -Events: on_app_create_completion -*/ - -// newCreateLogger creates a logger instance to receive event notifications -func newCreateLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_app_create_template_default": - printAppCreateDefaultemplate(cmd, event) - case "on_app_create_template_custom": - printAppCreateCustomTemplate(cmd, event) - case "on_app_create_completion": - printProjectCreateCompletion(clients, cmd, event) - default: - // Ignore the event - } - }, - ) -} - -/* -App creation (not Create command) is cloning the template and creating the project directory -Events: on_app_create_template_custom, on_app_create_completion -*/ - -func printAppCreateDefaultemplate(cmd *cobra.Command, event *logger.LogEvent) { - startAppCreateSpinner(copyTemplate) -} - -// Print template URL if using custom app template -func printAppCreateCustomTemplate(cmd *cobra.Command, event *logger.LogEvent) { - var verb string - templatePath := event.DataToString("templatePath") - isGit := event.DataToBool("isGit") - gitBranch := event.DataToString("gitBranch") - - if isGit { - verb = cloneTemplate - } else { - verb = copyTemplate - } - templateText := fmt.Sprintf( - "%s template from %s", - verb, - templatePath, - ) - - if gitBranch != "" { - templateText = fmt.Sprintf("%s (branch: %s)", templateText, gitBranch) - } - - cmd.Print(style.Secondary(templateText), "\n\n") - - startAppCreateSpinner(verb) -} - -func startAppCreateSpinner(verb string) { - appCreateSpinner.Update(verb+" app template", "").Start() -} - -// Display message and added files at completion of app creation -func printProjectCreateCompletion(clients *shared.ClientFactory, cmd *cobra.Command, event *logger.LogEvent) { - createCompletionText := style.Sectionf(style.TextSection{ - Emoji: "gear", - Text: "Created project directory", - }) - appCreateSpinner.Update(createCompletionText, "").Stop() -} - // printCreateSuccess outputs an informative message after creating a new app func printCreateSuccess(ctx context.Context, clients *shared.ClientFactory, appPath string) { // Check if this is a Deno project to conditionally enable some features diff --git a/cmd/project/create_test.go b/cmd/project/create_test.go index 37d83102..64604d50 100644 --- a/cmd/project/create_test.go +++ b/cmd/project/create_test.go @@ -20,7 +20,6 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/create" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/slackerror" @@ -35,8 +34,8 @@ type CreateClientMock struct { mock.Mock } -func (m *CreateClientMock) Create(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, createArgs create.CreateArgs) (string, error) { - args := m.Called(ctx, clients, log, createArgs) +func (m *CreateClientMock) Create(ctx context.Context, clients *shared.ClientFactory, createArgs create.CreateArgs) (string, error) { + args := m.Called(ctx, clients, createArgs) return args.String(0), args.Error(1) } @@ -66,7 +65,7 @@ func TestCreateCommand(t *testing.T) { cm.IO.On("InputPrompt", mock.Anything, "Name your app:", mock.Anything). Return("my-app", nil) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { @@ -76,7 +75,7 @@ func TestCreateCommand(t *testing.T) { AppName: "my-app", Template: template, } - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, expected) + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, expected) cm.IO.AssertCalled(t, "InputPrompt", mock.Anything, "Name your app:", mock.Anything) }, }, @@ -103,7 +102,7 @@ func TestCreateCommand(t *testing.T) { cm.IO.On("InputPrompt", mock.Anything, "Name your app:", mock.Anything). Return("my-deno-app", nil) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { @@ -113,7 +112,7 @@ func TestCreateCommand(t *testing.T) { AppName: "my-deno-app", Template: template, } - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, expected) + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, expected) cm.IO.AssertCalled(t, "InputPrompt", mock.Anything, "Name your app:", mock.Anything) }, }, @@ -133,7 +132,7 @@ func TestCreateCommand(t *testing.T) { cm.IO.On("InputPrompt", mock.Anything, "Name your app:", mock.Anything). Return("my-agent", nil) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { @@ -143,7 +142,7 @@ func TestCreateCommand(t *testing.T) { AppName: "my-agent", Template: template, } - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, expected) + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, expected) // Verify that category prompt was NOT called cm.IO.AssertNotCalled(t, "SelectPrompt", mock.Anything, "Select an app:", mock.Anything, mock.Anything) cm.IO.AssertCalled(t, "InputPrompt", mock.Anything, "Name your app:", mock.Anything) @@ -162,7 +161,7 @@ func TestCreateCommand(t *testing.T) { nil, ) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { @@ -172,7 +171,7 @@ func TestCreateCommand(t *testing.T) { AppName: "my-agent-app", Template: template, } - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, expected) + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, expected) // Verify that category prompt was NOT called cm.IO.AssertNotCalled(t, "SelectPrompt", mock.Anything, "Select an app:", mock.Anything, mock.Anything) // Verify that name prompt was NOT called since name was provided as arg @@ -199,7 +198,7 @@ func TestCreateCommand(t *testing.T) { nil, ) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { @@ -209,7 +208,7 @@ func TestCreateCommand(t *testing.T) { AppName: "agent", Template: template, } - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, expected) + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, expected) // Verify that name prompt was NOT called since name was provided as arg cm.IO.AssertNotCalled(t, "InputPrompt", mock.Anything, "Name your app:", mock.Anything) }, @@ -235,7 +234,7 @@ func TestCreateCommand(t *testing.T) { nil, ) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { @@ -245,7 +244,7 @@ func TestCreateCommand(t *testing.T) { AppName: "agent", Template: template, } - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, expected) + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, expected) // Verify that category prompt WAS called (shortcut was not triggered) cm.IO.AssertCalled(t, "SelectPrompt", mock.Anything, "Select an app:", mock.Anything, mock.Anything) // Verify that name prompt was NOT called since --name flag was provided @@ -265,7 +264,7 @@ func TestCreateCommand(t *testing.T) { nil, ) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { @@ -275,7 +274,7 @@ func TestCreateCommand(t *testing.T) { AppName: "my-custom-name", // --name flag overrides Template: template, } - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, expected) + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, expected) // Verify that category prompt was NOT called (shortcut was triggered) cm.IO.AssertNotCalled(t, "SelectPrompt", mock.Anything, "Select an app:", mock.Anything, mock.Anything) }, @@ -300,7 +299,7 @@ func TestCreateCommand(t *testing.T) { nil, ) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { @@ -310,7 +309,7 @@ func TestCreateCommand(t *testing.T) { AppName: "my-name", // --name flag overrides "my-project" positional arg Template: template, } - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, expected) + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, expected) // Verify that name prompt was NOT called since --name flag was provided cm.IO.AssertNotCalled(t, "InputPrompt", mock.Anything, "Name your app:", mock.Anything) }, @@ -328,7 +327,7 @@ func TestCreateCommand(t *testing.T) { nil, ) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { @@ -338,7 +337,7 @@ func TestCreateCommand(t *testing.T) { AppName: "my-name", // --name flag overrides "my-project" positional arg Template: template, } - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, expected) + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, expected) // Verify that category prompt was NOT called (agent shortcut was triggered) cm.IO.AssertNotCalled(t, "SelectPrompt", mock.Anything, "Select an app:", mock.Anything, mock.Anything) // Verify that name prompt was NOT called since --name flag was provided @@ -368,13 +367,13 @@ func TestCreateCommand(t *testing.T) { cm.IO.On("InputPrompt", mock.Anything, "Name your app:", mock.Anything). Return("", nil) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.IO.AssertCalled(t, "InputPrompt", mock.Anything, "Name your app:", mock.Anything) // When the user accepts the default (empty return), the generated name is used - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(args create.CreateArgs) bool { + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.MatchedBy(func(args create.CreateArgs) bool { return args.AppName != "" })) }, @@ -400,14 +399,14 @@ func TestCreateCommand(t *testing.T) { nil, ) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { // Should NOT prompt for name since not a TTY cm.IO.AssertNotCalled(t, "InputPrompt", mock.Anything, "Name your app:", mock.Anything) // Should still call Create with a non-empty generated name - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(args create.CreateArgs) bool { + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.MatchedBy(func(args create.CreateArgs) bool { return args.AppName != "" })) }, @@ -432,7 +431,7 @@ func TestCreateCommand(t *testing.T) { nil, ) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { @@ -442,7 +441,7 @@ func TestCreateCommand(t *testing.T) { AppName: "my-project", Template: template, } - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, expected) + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, expected) // Verify that name prompt was NOT called since name was provided as positional arg cm.IO.AssertNotCalled(t, "InputPrompt", mock.Anything, "Name your app:", mock.Anything) }, @@ -455,7 +454,7 @@ func TestCreateCommand(t *testing.T) { }, ExpectedErrorStrings: []string{"The --subdir flag requires the --template flag"}, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - createClientMock.AssertNotCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + createClientMock.AssertNotCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything) }, }, "passes subdir flag to create function": { @@ -478,13 +477,13 @@ func TestCreateCommand(t *testing.T) { nil, ) createClientMock = new(CreateClientMock) - createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", nil) + createClientMock.On("Create", mock.Anything, mock.Anything, mock.Anything).Return("", nil) CreateFunc = createClientMock.Create }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { template, err := create.ResolveTemplateURL("slack-samples/bolt-js-starter-template") require.NoError(t, err) - createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(args create.CreateArgs) bool { + createClientMock.AssertCalled(t, "Create", mock.Anything, mock.Anything, mock.MatchedBy(func(args create.CreateArgs) bool { return args.AppName != "" && args.Template == template && args.Subdir == "apps/my-app" })) }, @@ -499,7 +498,7 @@ func TestCreateCommand(t *testing.T) { "Getting started", }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - createClientMock.AssertNotCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + createClientMock.AssertNotCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything) }, }, "lists all templates with --list flag": { @@ -521,7 +520,7 @@ func TestCreateCommand(t *testing.T) { "slack-samples/deno-starter-template", }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - createClientMock.AssertNotCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + createClientMock.AssertNotCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything) }, }, "lists agent templates with agent --list flag": { @@ -536,7 +535,7 @@ func TestCreateCommand(t *testing.T) { "slack-samples/bolt-python-assistant-template", }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - createClientMock.AssertNotCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + createClientMock.AssertNotCalled(t, "Create", mock.Anything, mock.Anything, mock.Anything) output := cm.GetCombinedOutput() assert.NotContains(t, output, "Getting started") assert.NotContains(t, output, "Automation apps") diff --git a/cmd/project/samples_test.go b/cmd/project/samples_test.go index c3eedbae..3cdc3d3d 100644 --- a/cmd/project/samples_test.go +++ b/cmd/project/samples_test.go @@ -19,7 +19,6 @@ import ( "testing" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" createPkg "github.com/slackapi/slack-cli/internal/pkg/create" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/test/testutil" @@ -72,7 +71,7 @@ func TestSamplesCommand(t *testing.T) { }, nil, ) - CreateFunc = func(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, createArgs createPkg.CreateArgs) (appDirPath string, err error) { + CreateFunc = func(ctx context.Context, clients *shared.ClientFactory, createArgs createPkg.CreateArgs) (appDirPath string, err error) { return createArgs.AppName, nil } }, diff --git a/internal/pkg/create/create.go b/internal/pkg/create/create.go index ee079fb3..079626ff 100644 --- a/internal/pkg/create/create.go +++ b/internal/pkg/create/create.go @@ -34,7 +34,6 @@ import ( "github.com/slackapi/slack-cli/internal/deputil" "github.com/slackapi/slack-cli/internal/experiment" "github.com/slackapi/slack-cli/internal/goutils" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/slackhttp" @@ -58,7 +57,7 @@ type CreateArgs struct { } // Create will create a new Slack app on the file system and app manifest on the Slack API. -func Create(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, createArgs CreateArgs) (appDirPath string, err error) { +func Create(ctx context.Context, clients *shared.ClientFactory, createArgs CreateArgs) (appDirPath string, err error) { span, ctx := opentracing.StartSpanFromContext(ctx, "cmd.create") defer span.Finish() @@ -132,11 +131,11 @@ func Create(ctx context.Context, clients *shared.ClientFactory, log *logger.Logg } if subdir != "" { - if err := createAppFromSubdir(ctx, projectDirPath, createArgs.Template, createArgs.GitBranch, subdir, log, clients.Fs); err != nil { + if err := createAppFromSubdir(ctx, projectDirPath, createArgs.Template, createArgs.GitBranch, subdir, clients.Fs); err != nil { return "", slackerror.Wrap(err, slackerror.ErrAppCreate) } } else { - if err := createApp(ctx, projectDirPath, createArgs.Template, createArgs.GitBranch, log, clients.Fs); err != nil { + if err := createApp(ctx, projectDirPath, createArgs.Template, createArgs.GitBranch, clients.Fs); err != nil { return "", slackerror.Wrap(err, slackerror.ErrAppCreate) } } @@ -160,9 +159,6 @@ func Create(ctx context.Context, clients *shared.ClientFactory, log *logger.Logg InstallProjectDependencies(ctx, clients, projectDirPath, config.ManifestSourceLocal) clients.IO.PrintTrace(ctx, slacktrace.CreateDependenciesSuccess) - // Notify listeners that app directory is created - log.Log("info", "on_app_create_completion") - return appDirPath, nil } @@ -222,15 +218,7 @@ func parentDirExists(dirPath string) (bool, error) { } // createApp will create the app directory using the default app template or a specified template URL. -func createApp(ctx context.Context, dirPath string, template Template, gitBranch string, log *logger.Logger, fs afero.Fs) error { - log.Data["templatePath"] = template.path - log.Data["isGit"] = template.isGit - log.Data["gitBranch"] = gitBranch - log.Data["isSample"] = template.IsSample() - - // Notify listeners - log.Log("info", "on_app_create_template") - +func createApp(ctx context.Context, dirPath string, template Template, gitBranch string, fs afero.Fs) error { if template.isGit { doctorSection, err := doctor.CheckGit(ctx) if doctorSection.HasError() || err != nil { @@ -369,7 +357,7 @@ func normalizeSubdir(subdir string) (string, error) { // createAppFromSubdir clones the full template into a temp directory, then copies // only the specified subdirectory to the final project path. -func createAppFromSubdir(ctx context.Context, dirPath string, template Template, gitBranch string, subdir string, log *logger.Logger, fs afero.Fs) error { +func createAppFromSubdir(ctx context.Context, dirPath string, template Template, gitBranch string, subdir string, fs afero.Fs) error { tmpDirRoot := afero.GetTempDir(fs, "") tmpDir, err := afero.TempDir(fs, tmpDirRoot, "slack-create-") if err != nil { @@ -378,7 +366,7 @@ func createAppFromSubdir(ctx context.Context, dirPath string, template Template, defer func() { _ = fs.RemoveAll(tmpDir) }() cloneDir := filepath.Join(tmpDir, "repo") - if err := createApp(ctx, cloneDir, template, gitBranch, log, fs); err != nil { + if err := createApp(ctx, cloneDir, template, gitBranch, fs); err != nil { return err } diff --git a/internal/pkg/create/create_test.go b/internal/pkg/create/create_test.go index baaa5583..37ff3063 100644 --- a/internal/pkg/create/create_test.go +++ b/internal/pkg/create/create_test.go @@ -22,7 +22,6 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/experiment" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackhttp" @@ -301,9 +300,8 @@ func TestCreateAppFromSubdir(t *testing.T) { require.NoError(t, fs.Remove(outputDir)) template := Template{path: templateDir, isLocal: true} - log := logger.New(func(event *logger.LogEvent) {}) - err := createAppFromSubdir(t.Context(), outputDir, template, "", tc.subdir, log, fs) + err := createAppFromSubdir(t.Context(), outputDir, template, "", tc.subdir, fs) if tc.expectError { assert.Error(t, err) From ea5b1668c61068036b46e9eae434a937e23fe1ca Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 5 Mar 2026 16:46:50 -0800 Subject: [PATCH 3/5] refactor: remove logger from app install, add, and function mocks --- cmd/app/add.go | 68 ++-------------------- cmd/function/mocks_test.go | 9 ++- internal/pkg/apps/add.go | 24 +++----- internal/pkg/apps/install.go | 84 ++++++++++++++++------------ internal/pkg/apps/install_test.go | 5 -- internal/pkg/platform/localserver.go | 2 +- internal/pkg/platform/run.go | 2 +- 7 files changed, 65 insertions(+), 129 deletions(-) diff --git a/cmd/app/add.go b/cmd/app/add.go index 39b4623c..a63a0e15 100644 --- a/cmd/app/add.go +++ b/cmd/app/add.go @@ -16,13 +16,11 @@ package app import ( "context" - "fmt" "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/experiment" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/apps" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -186,11 +184,8 @@ func RunAddCommand(ctx context.Context, clients *shared.ClientFactory, selection clients.Config.ManifestEnv = app.SetManifestEnvTeamVars(clients.Config.ManifestEnv, selection.App.TeamDomain, selection.App.IsDev) - // Set up event logger - log := newAddLogger(clients, selection.Auth.TeamDomain) - // Install dev app or prod app to a workspace - installedApp, installState, err := appInstall(ctx, clients, log, selection, orgGrantWorkspaceID) + installedApp, installState, err := appInstall(ctx, clients, selection, orgGrantWorkspaceID) if err != nil { return ctx, installState, types.App{}, err // pass the installState because some callers may use it to handle the error } @@ -201,74 +196,19 @@ func RunAddCommand(ctx context.Context, clients *shared.ClientFactory, selection return ctx, installState, installedApp, nil } -// newAddLogger creates a logger instance to receive event notifications -func newAddLogger(clients *shared.ClientFactory, envName string) *logger.Logger { - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - teamName := event.DataToString("teamName") - appName := event.DataToString("appName") - switch event.Name { - case "app_install_manifest": - // Ignore this event and format manifest outputs in create/update events - case "app_install_manifest_create": - _, _ = clients.IO.WriteOut().Write([]byte(style.Sectionf(style.TextSection{ - Emoji: "books", - Text: "App Manifest", - Secondary: []string{ - fmt.Sprintf(`Creating app manifest for "%s" in "%s"`, appName, teamName), - }, - }))) - case "app_install_manifest_update": - _, _ = clients.IO.WriteOut().Write([]byte("\n" + style.Sectionf(style.TextSection{ - Emoji: "books", - Text: "App Manifest", - Secondary: []string{ - fmt.Sprintf(`Updated app manifest for "%s" in "%s"`, appName, teamName), - }, - }))) - case "app_install_start": - _, _ = clients.IO.WriteOut().Write([]byte("\n" + style.Sectionf(style.TextSection{ - Emoji: "house", - Text: "App Install", - Secondary: []string{ - fmt.Sprintf(`Installing "%s" app to "%s"`, appName, teamName), - }, - }))) - case "app_install_icon_success": - iconPath := event.DataToString("iconPath") - _, _ = clients.IO.WriteOut().Write([]byte( - style.SectionSecondaryf("Updated app icon: %s", iconPath), - )) - case "app_install_icon_error": - iconError := event.DataToString("iconError") - _, _ = clients.IO.WriteOut().Write([]byte( - style.SectionSecondaryf("Error updating app icon: %s", iconError), - )) - case "app_install_complete": - _, _ = clients.IO.WriteOut().Write([]byte( - style.SectionSecondaryf("Finished in %s", event.DataToString("installTime")), - )) - default: - // Ignore the event - } - }, - ) -} - // printAddSuccess will print a list of the environments func printAddSuccess(clients *shared.ClientFactory, cmd *cobra.Command, appInstance types.App) error { return runListCommand(cmd, clients) } // appInstall will install an app to a team. It supports both local and deployed app types. -func appInstall(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, selection *prompts.SelectedApp, orgGrantWorkspaceID string) (types.App, types.InstallState, error) { +func appInstall(ctx context.Context, clients *shared.ClientFactory, selection *prompts.SelectedApp, orgGrantWorkspaceID string) (types.App, types.InstallState, error) { if selection != nil && selection.App.IsDev { // Install local dev app to a team - installedApp, _, installState, err := appInstallDevAppFunc(ctx, clients, "", log, selection.Auth, selection.App) + installedApp, _, installState, err := appInstallDevAppFunc(ctx, clients, "", selection.Auth, selection.App) return installedApp, installState, err } else { - installState, installedApp, err := appInstallProdAppFunc(ctx, clients, log, selection.Auth, selection.App, orgGrantWorkspaceID) + installState, installedApp, err := appInstallProdAppFunc(ctx, clients, selection.Auth, selection.App, orgGrantWorkspaceID) return installedApp, installState, err } } diff --git a/cmd/function/mocks_test.go b/cmd/function/mocks_test.go index b8e3c64d..fdf30eda 100644 --- a/cmd/function/mocks_test.go +++ b/cmd/function/mocks_test.go @@ -17,7 +17,6 @@ package function_test import ( "context" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/stretchr/testify/mock" @@ -27,22 +26,22 @@ type FunctionDistributorMock struct { mock.Mock } -func (m *FunctionDistributorMock) List(ctx context.Context, clients *shared.ClientFactory, fn string, log *logger.Logger) (types.Permission, []types.FunctionDistributionUser, error) { +func (m *FunctionDistributorMock) List(ctx context.Context, clients *shared.ClientFactory, fn string) (types.Permission, []types.FunctionDistributionUser, error) { args := m.Called() return args.Get(0).(types.Permission), args.Get(1).([]types.FunctionDistributionUser), args.Error(2) } -func (m *FunctionDistributorMock) Set(ctx context.Context, clients *shared.ClientFactory, function, distributionType, users string, log *logger.Logger) (string, error) { +func (m *FunctionDistributorMock) Set(ctx context.Context, clients *shared.ClientFactory, function, distributionType, users string) (string, error) { args := m.Called() return args.String(0), args.Error(1) } -func (m *FunctionDistributorMock) AddUsers(ctx context.Context, clients *shared.ClientFactory, function, users string, log *logger.Logger) (string, error) { +func (m *FunctionDistributorMock) AddUsers(ctx context.Context, clients *shared.ClientFactory, function, users string) (string, error) { args := m.Called() return args.String(0), args.Error(1) } -func (m *FunctionDistributorMock) RemoveUsers(ctx context.Context, clients *shared.ClientFactory, function, users string, log *logger.Logger) (string, error) { +func (m *FunctionDistributorMock) RemoveUsers(ctx context.Context, clients *shared.ClientFactory, function, users string) (string, error) { args := m.Called() return args.String(0), args.Error(1) } diff --git a/internal/pkg/apps/add.go b/internal/pkg/apps/add.go index 421fa940..81e2462d 100644 --- a/internal/pkg/apps/add.go +++ b/internal/pkg/apps/add.go @@ -19,35 +19,31 @@ import ( "strings" "github.com/opentracing/opentracing-go" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" ) // Add will add an app -func Add(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, auth types.SlackAuth, app types.App, orgGrantWorkspaceID string) (types.InstallState, types.App, error) { +func Add(ctx context.Context, clients *shared.ClientFactory, auth types.SlackAuth, app types.App, orgGrantWorkspaceID string) (types.InstallState, types.App, error) { span, _ := opentracing.StartSpanFromContext(ctx, "pkg.apps.add") defer span.Finish() // Validate the auth - ctx, authSession, err := getAuthSession(ctx, clients, auth) + ctx, _, err := getAuthSession(ctx, clients, auth) if err != nil { return "", types.App{}, slackerror.Wrap(err, slackerror.ErrAddAppToProject) } - log.Data["teamName"] = *authSession.TeamName - - log.Info("on_apps_add_init") // Add app remotely via Slack API - installState, app, err := addAppRemotely(ctx, clients, log, auth, app, orgGrantWorkspaceID) + installState, app, err := addAppRemotely(ctx, clients, auth, app, orgGrantWorkspaceID) if err != nil { return "", types.App{}, slackerror.Wrap(err, slackerror.ErrAddAppToProject) } // Add app to apps.json if !clients.Config.SkipLocalFs() { - if _, err := addAppLocally(ctx, clients, log, app); err != nil { + if _, err := addAppLocally(ctx, clients, app); err != nil { return installState, types.App{}, slackerror.Wrap(err, slackerror.ErrAddAppToProject) } } @@ -56,9 +52,7 @@ func Add(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, } // addAppLocally will add the app to the project's apps file with an empty AppID, TeamID, etc -func addAppLocally(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, app types.App) (types.App, error) { - log.Info("on_apps_add_local") - +func addAppLocally(ctx context.Context, clients *shared.ClientFactory, app types.App) (types.App, error) { app, err := clients.AppClient().NewDeployed(ctx, app.TeamID) if err != nil { if !strings.Contains(err.Error(), slackerror.ErrAppFound) { // Ignore the error when the app already exists @@ -69,22 +63,18 @@ func addAppLocally(ctx context.Context, clients *shared.ClientFactory, log *logg } // addAppRemotely will create the app manifest using the current auth account's team -func addAppRemotely(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, auth types.SlackAuth, app types.App, orgGrantWorkspaceID string) (types.InstallState, types.App, error) { - log.Info("on_apps_add_remote_init") - +func addAppRemotely(ctx context.Context, clients *shared.ClientFactory, auth types.SlackAuth, app types.App, orgGrantWorkspaceID string) (types.InstallState, types.App, error) { if app.TeamID == "" { // App hasn't been created yet and // so the target team ID set by the auth app.TeamID = auth.TeamID } - app, installState, err := Install(ctx, clients, log, auth, CreateAppManifestAndInstall, app, orgGrantWorkspaceID) + app, installState, err := Install(ctx, clients, auth, CreateAppManifestAndInstall, app, orgGrantWorkspaceID) if err != nil { return installState, types.App{}, slackerror.Wrap(err, slackerror.ErrAppAdd) } - log.Info("on_apps_add_remote_success") - if !clients.Config.SkipLocalFs() { app, err = clients.AppClient().GetDeployed(ctx, app.TeamID) } diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 7bfe922a..731a067d 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -25,7 +25,6 @@ import ( "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/logger" "github.com/slackapi/slack-cli/internal/pkg/manifest" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" @@ -42,7 +41,7 @@ const ( const additionalManifestInfoNotice = "App manifest contains some components that may require additional information" // Install installs the app to a team -func Install(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, auth types.SlackAuth, onlyCreateUpdateAppManifest bool, app types.App, orgGrantWorkspaceID string) (types.App, types.InstallState, error) { +func Install(ctx context.Context, clients *shared.ClientFactory, auth types.SlackAuth, onlyCreateUpdateAppManifest bool, app types.App, orgGrantWorkspaceID string) (types.App, types.InstallState, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.apps.install") defer span.Finish() @@ -114,11 +113,6 @@ func Install(ctx context.Context, clients *shared.ClientFactory, log *logger.Log } } - log.Data["appName"] = slackManifest.DisplayInformation.Name - log.Data["isUpdate"] = app.AppID != "" - log.Data["teamName"] = *authSession.TeamName - log.Log("INFO", "app_install_manifest") - manifest := slackManifest.AppManifest if slackManifest.IsFunctionRuntimeSlackHosted() { configureHostedManifest(ctx, clients, &manifest) @@ -132,14 +126,26 @@ func Install(ctx context.Context, clients *shared.ClientFactory, log *logger.Log start := time.Now() switch { case manifestUpdates: - log.Info("app_install_manifest_update") + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "books", + Text: "App Manifest", + Secondary: []string{ + fmt.Sprintf(`Updated app manifest for "%s" in "%s"`, slackManifest.DisplayInformation.Name, *authSession.TeamName), + }, + })) clients.IO.PrintDebug(ctx, "updating app %s", app.AppID) _, err := apiInterface.UpdateApp(ctx, token, app.AppID, manifest, clients.Config.ForceFlag, true) if err != nil { return app, "", err } case manifestCreates: - log.Info("app_install_manifest_create") + clients.IO.PrintInfo(ctx, false, "%s", style.Sectionf(style.TextSection{ + Emoji: "books", + Text: "App Manifest", + Secondary: []string{ + fmt.Sprintf(`Creating app manifest for "%s" in "%s"`, slackManifest.DisplayInformation.Name, *authSession.TeamName), + }, + })) clients.IO.PrintDebug(ctx, "app not found so creating a new app") result, err := apiInterface.CreateApp(ctx, token, manifest, false) if err != nil { @@ -156,10 +162,6 @@ func Install(ctx context.Context, clients *shared.ClientFactory, log *logger.Log // app.EnterpriseID = config.GetContextEnterpriseID(ctx) } - appManageURL := fmt.Sprintf("%s/apps", apiInterface.Host()) - log.Data["appURL"] = fmt.Sprintf("%s%s", appManageURL, app.AppID) - log.Data["appName"] = manifest.DisplayInformation.Name - if !clients.Config.SkipLocalFs() { if err := clients.AppClient().SaveDeployed(ctx, app); err != nil { return types.App{}, "", err @@ -205,7 +207,13 @@ func Install(ctx context.Context, clients *shared.ClientFactory, log *logger.Log outgoingDomains = *manifest.OutgoingDomains } - log.Info("app_install_start") + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "house", + Text: "App Install", + Secondary: []string{ + fmt.Sprintf(`Installing "%s" app to "%s"`, manifest.DisplayInformation.Name, *authSession.TeamName), + }, + })) // Note - we use DeveloperAppInstall endpoint for both local (dev) runs // and hosted installs https://github.com/slackapi/slack-cli/pull/456#discussion_r830272175 @@ -234,14 +242,12 @@ func Install(ctx context.Context, clients *shared.ClientFactory, log *logger.Log } } if iconPath != "" { - log.Data["iconPath"] = iconPath err = updateIcon(ctx, clients, iconPath, app.AppID, token) if err != nil { clients.IO.PrintDebug(ctx, "icon error: %s", err) - log.Data["iconError"] = err.Error() - log.Info("app_install_icon_error") + clients.IO.PrintInfo(ctx, false, "%s", style.SectionSecondaryf("Error updating app icon: %s", err)) } else { - log.Info("app_install_icon_success") + clients.IO.PrintInfo(ctx, false, "%s", style.SectionSecondaryf("Updated app icon: %s", iconPath)) } // TODO: Optimization. // Save a md5 hash of the icon in environments.yaml @@ -255,8 +261,7 @@ func Install(ctx context.Context, clients *shared.ClientFactory, log *logger.Log // update config with latest yaml hash // env.Hash = slackYaml.Hash - log.Data["installTime"] = fmt.Sprintf("%.1fs", time.Since(start).Seconds()) - log.Info("app_install_complete") + clients.IO.PrintInfo(ctx, false, "%s", style.SectionSecondaryf("Finished in %.1fs", time.Since(start).Seconds())) return app, types.InstallSuccess, nil } @@ -356,7 +361,7 @@ func validateManifestForInstall(ctx context.Context, clients *shared.ClientFacto } // InstallLocalApp installs a non-hosted local app to a workspace. -func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGrantWorkspaceID string, log *logger.Logger, auth types.SlackAuth, app types.App) (types.App, api.DeveloperAppInstallResult, types.InstallState, error) { +func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGrantWorkspaceID string, auth types.SlackAuth, app types.App) (types.App, api.DeveloperAppInstallResult, types.InstallState, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "installLocalApp") defer span.Finish() @@ -428,11 +433,6 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran } } - log.Data["appName"] = slackManifest.DisplayInformation.Name - log.Data["isUpdate"] = app.AppID != "" - log.Data["teamName"] = *authSession.TeamName - log.Log("INFO", "app_install_manifest") - manifest := slackManifest.AppManifest appendLocalToDisplayName(&manifest) if manifest.IsFunctionRuntimeSlackHosted() { @@ -447,8 +447,13 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran start := time.Now() switch { case manifestUpdates: - log.Info("app_install_manifest_update") - log.Info("on_update_app_install") + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "books", + Text: "App Manifest", + Secondary: []string{ + fmt.Sprintf(`Updated app manifest for "%s" in "%s"`, slackManifest.DisplayInformation.Name, *authSession.TeamName), + }, + })) clients.IO.PrintDebug(ctx, "updating app %s", app.AppID) _, err := apiInterface.UpdateApp(ctx, token, app.AppID, manifest, clients.Config.ForceFlag, true) if err != nil { @@ -456,7 +461,13 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran return app, api.DeveloperAppInstallResult{}, "", err } case manifestCreates: - log.Info("app_install_manifest_create") + clients.IO.PrintInfo(ctx, false, "%s", style.Sectionf(style.TextSection{ + Emoji: "books", + Text: "App Manifest", + Secondary: []string{ + fmt.Sprintf(`Creating app manifest for "%s" in "%s"`, slackManifest.DisplayInformation.Name, *authSession.TeamName), + }, + })) clients.IO.PrintDebug(ctx, "app not found so creating a new app") result, err := apiInterface.CreateApp(ctx, token, manifest, false) if err != nil { @@ -474,10 +485,6 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran app.UserID = *authSession.UserID } - appManageURL := fmt.Sprintf("%s/apps", apiInterface.Host()) - log.Data["appURL"] = fmt.Sprintf("%s%s", appManageURL, app.AppID) - log.Data["appName"] = manifest.DisplayInformation.Name - // specifically set app.IsDev to be true for dev installation app.IsDev = true @@ -523,7 +530,13 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran outgoingDomains = *manifest.OutgoingDomains } - log.Info("app_install_start") + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "house", + Text: "App Install", + Secondary: []string{ + fmt.Sprintf(`Installing "%s" app to "%s"`, manifest.DisplayInformation.Name, *authSession.TeamName), + }, + })) var installState types.InstallState result, installState, err := apiInterface.DeveloperAppInstall(ctx, clients.IO, token, app, botScopes, outgoingDomains, orgGrantWorkspaceID, clients.Config.AutoRequestAAAFlag) @@ -569,8 +582,7 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran // update config with latest yaml hash // env.Hash = slackYaml.Hash - log.Data["installTime"] = fmt.Sprintf("%.1fs", time.Since(start).Seconds()) - log.Info("app_install_complete") + clients.IO.PrintInfo(ctx, false, "%s", style.SectionSecondaryf("Finished in %.1fs", time.Since(start).Seconds())) return app, result, types.InstallSuccess, nil } diff --git a/internal/pkg/apps/install_test.go b/internal/pkg/apps/install_test.go index 60503614..bf0b190c 100644 --- a/internal/pkg/apps/install_test.go +++ b/internal/pkg/apps/install_test.go @@ -23,7 +23,6 @@ import ( "github.com/slackapi/slack-cli/internal/cache" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/experiment" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" @@ -625,12 +624,10 @@ func TestInstall(t *testing.T) { mockProjectConfig.On("Cache").Return(mockProjectCache) clientsMock.Config.ProjectConfig = mockProjectConfig - log := logger.New(func(event *logger.LogEvent) {}) clients := shared.NewClientFactory(clientsMock.MockClientFactory()) app, state, err := Install( ctx, clients, - log, tc.mockAuth, false, tc.mockApp, @@ -1453,13 +1450,11 @@ func TestInstallLocalApp(t *testing.T) { mockProjectConfig.On("Cache").Return(mockProjectCache) clientsMock.Config.ProjectConfig = mockProjectConfig - log := logger.New(func(event *logger.LogEvent) {}) clients := shared.NewClientFactory(clientsMock.MockClientFactory()) app, _, state, err := InstallLocalApp( ctx, clients, tc.mockOrgGrantWorkspaceID, - log, tc.mockAuth, tc.mockApp, ) diff --git a/internal/pkg/platform/localserver.go b/internal/pkg/platform/localserver.go index b5e20a6b..45365577 100644 --- a/internal/pkg/platform/localserver.go +++ b/internal/pkg/platform/localserver.go @@ -414,7 +414,7 @@ func (r *LocalServer) WatchManifest(ctx context.Context, auth types.SlackAuth, a r.log.Info("on_cloud_run_watch_manifest_change") // Reinstall the app when manifest changes - if _, _, _, err := apps.InstallLocalApp(ctx, r.clients, "", r.log, auth, app); err != nil { + if _, _, _, err := apps.InstallLocalApp(ctx, r.clients, "", auth, app); err != nil { r.log.Data["cloud_run_watch_error"] = err.Error() r.log.Warn("on_cloud_run_watch_error") } else { diff --git a/internal/pkg/platform/run.go b/internal/pkg/platform/run.go index f6015cda..6a6d707c 100644 --- a/internal/pkg/platform/run.go +++ b/internal/pkg/platform/run.go @@ -77,7 +77,7 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, } // Update local install - installedApp, localInstallResult, installState, err := apps.InstallLocalApp(ctx, clients, runArgs.OrgGrantWorkspaceID, log, runArgs.Auth, runArgs.App) + installedApp, localInstallResult, installState, err := apps.InstallLocalApp(ctx, clients, runArgs.OrgGrantWorkspaceID, runArgs.Auth, runArgs.App) if err != nil { return nil, "", slackerror.Wrap(err, slackerror.ErrLocalAppRun) } From a2b971d95cf4a56f641f70425fb328dd265eb3bb Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 5 Mar 2026 17:02:09 -0800 Subject: [PATCH 4/5] refactor: remove logger from platform run --- cmd/platform/run.go | 61 +---------------------- cmd/platform/run_test.go | 13 +++-- internal/pkg/platform/localserver.go | 35 +++++-------- internal/pkg/platform/localserver_test.go | 9 ---- internal/pkg/platform/run.go | 40 +++++++-------- 5 files changed, 40 insertions(+), 118 deletions(-) diff --git a/cmd/platform/run.go b/cmd/platform/run.go index 1ae05a1f..bcd8f12a 100644 --- a/cmd/platform/run.go +++ b/cmd/platform/run.go @@ -21,12 +21,10 @@ import ( "github.com/slackapi/slack-cli/cmd/triggers" internalapp "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/cmdutil" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/platform" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/slackerror" - "github.com/slackapi/slack-cli/internal/slacktrace" "github.com/slackapi/slack-cli/internal/style" "github.com/spf13/cobra" ) @@ -144,68 +142,11 @@ func RunRunCommand(clients *shared.ClientFactory, cmd *cobra.Command, args []str OrgGrantWorkspaceID: runFlags.orgGrantWorkspaceID, } - log := newRunLogger(clients, cmd) - // Run dev app locally - if _, _, err := runFunc(ctx, clients, log, runArgs); err != nil { + if _, err := runFunc(ctx, clients, runArgs); err != nil { return err } return nil } -// newRunLogger creates a logger instance to receive event notifications -func newRunLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger { - ctx := cmd.Context() - return logger.New( - // OnEvent - func(event *logger.LogEvent) { - switch event.Name { - case "on_update_app_install": - cmd.Println(style.Secondary(fmt.Sprintf( - `Updating local app install for "%s"`, - event.DataToString("teamName"), - ))) - case "on_cloud_run_connection_connected": - clients.IO.PrintTrace(ctx, slacktrace.PlatformRunReady) - cmd.Println(style.Secondary("Connected, awaiting events")) - case "on_cloud_run_connection_message": - message := event.DataToString("cloud_run_connection_message") - clients.IO.PrintDebug(ctx, "received: %s", message) - case "on_cloud_run_connection_command_error": - message := event.DataToString("cloud_run_connection_command_error") - clients.IO.PrintError(ctx, "Error: %s", message) - case "on_cloud_run_watch_error": - message := event.DataToString("cloud_run_watch_error") - clients.IO.PrintError(ctx, "Error: %s", message) - case "on_cloud_run_watch_manifest_change": - path := event.DataToString("cloud_run_watch_manifest_change") - cmd.Println(style.Secondary(fmt.Sprintf("Manifest change detected: %s, reinstalling app...", path))) - case "on_cloud_run_watch_manifest_change_reinstalled": - cmd.Println(style.Secondary("App successfully reinstalled")) - case "on_cloud_run_watch_manifest_change_skipped_remote": - path := event.DataToString("cloud_run_watch_manifest_change_skipped") - cmd.Println(style.Secondary(fmt.Sprintf("Manifest change detected: %s, skipped reinstalling app because manifest.source=remote", path))) - case "on_cloud_run_watch_app_change": - path := event.DataToString("cloud_run_watch_app_change") - cmd.Println(style.Secondary(fmt.Sprintf("App change detected: %s, restarting server...", path))) - case "on_cleanup_app_install_done": - cmd.Println(style.Secondary(fmt.Sprintf( - `Cleaned up local app install for "%s".`, - event.DataToString("teamName"), - ))) - case "on_cleanup_app_install_failed": - cmd.Println(style.Secondary(fmt.Sprintf( - `Cleaning up local app install for "%s" failed.`, - event.DataToString("teamName"), - ))) - message := event.DataToString("on_cleanup_app_install_error") - clients.IO.PrintWarning(ctx, "Local app cleanup failed: %s", message) - case "on_abort_cleanup_app_install": - cmd.Println(style.Secondary("Aborting, local app might not be cleaned up.")) - default: - // Ignore the event - } - }, - ) -} diff --git a/cmd/platform/run_test.go b/cmd/platform/run_test.go index 01ffdbfe..ef656c67 100644 --- a/cmd/platform/run_test.go +++ b/cmd/platform/run_test.go @@ -20,7 +20,6 @@ import ( "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/hooks" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/platform" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -48,9 +47,9 @@ type RunPkgMock struct { mock.Mock } -func (m *RunPkgMock) Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, runArgs platform.RunArgs) (*logger.LogEvent, types.InstallState, error) { - args := m.Called(ctx, clients, log, runArgs) - return log.SuccessEvent(), types.InstallSuccess, args.Error(0) +func (m *RunPkgMock) Run(ctx context.Context, clients *shared.ClientFactory, runArgs platform.RunArgs) (types.InstallState, error) { + args := m.Called(ctx, clients, runArgs) + return types.InstallSuccess, args.Error(0) } func TestRunCommand_Flags(t *testing.T) { @@ -222,7 +221,7 @@ func TestRunCommand_Flags(t *testing.T) { runPkgMock := new(RunPkgMock) runFunc = runPkgMock.Run - runPkgMock.On("Run", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + runPkgMock.On("Run", mock.Anything, mock.Anything, mock.Anything).Return(nil) cmd := NewRunCommand(clients) testutil.MockCmdIO(clients.IO, cmd) @@ -234,7 +233,7 @@ func TestRunCommand_Flags(t *testing.T) { // Check args passed into the run function if tc.expectedErr == nil { assert.NoError(t, err) - runPkgMock.AssertCalled(t, "Run", mock.Anything, mock.Anything, mock.Anything, + runPkgMock.AssertCalled(t, "Run", mock.Anything, mock.Anything, tc.expectedRunArgs, ) } else { @@ -256,7 +255,7 @@ func TestRunCommand_Help(t *testing.T) { runPkgMock := new(RunPkgMock) runFunc = runPkgMock.Run - runPkgMock.On("Run", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + runPkgMock.On("Run", mock.Anything, mock.Anything, mock.Anything).Return(nil) err := cmd.ExecuteContext(ctx) assert.NoError(t, err) diff --git a/internal/pkg/platform/localserver.go b/internal/pkg/platform/localserver.go index 45365577..ed1735bc 100644 --- a/internal/pkg/platform/localserver.go +++ b/internal/pkg/platform/localserver.go @@ -33,11 +33,12 @@ import ( "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/hooks" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/apps" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/slackapi/slack-cli/internal/slacktrace" + "github.com/slackapi/slack-cli/internal/style" ) // for lazy testing @@ -78,7 +79,6 @@ type LocalHostedContext struct { // response management to the script hook type LocalServer struct { clients *shared.ClientFactory - log *logger.Logger token string localHostedContext LocalHostedContext cliConfig hooks.SDKCLIConfig @@ -143,7 +143,8 @@ func (r *LocalServer) Start(ctx context.Context) error { // Listen waits for incoming events over a socket connection and invokes the deno-sdk-powered app with each payload. Responds to each event in a way that mimics the behaviour of a hosted app. func (r *LocalServer) Listen(ctx context.Context, errChan chan<- error, done chan<- bool) { - r.log.Info("on_cloud_run_connection_connected") + r.clients.IO.PrintTrace(ctx, slacktrace.PlatformRunReady) + r.clients.IO.PrintInfo(ctx, false, "%s", style.Secondary("Connected, awaiting events")) // Listen for socket messages for { @@ -169,8 +170,7 @@ func (r *LocalServer) Listen(ctx context.Context, errChan chan<- error, done cha } return } - r.log.Data["cloud_run_connection_message"] = string(messageBytes) - r.log.Debug("on_cloud_run_connection_message") + r.clients.IO.PrintDebug(ctx, "received: %s", string(messageBytes)) var msg Message err = json.Unmarshal(messageBytes, &msg) @@ -222,13 +222,10 @@ func (r *LocalServer) Listen(ctx context.Context, errChan chan<- error, done cha if err != nil { // Log the error but do not return because the user may be able to recover inside their app code - r.log.Data["cloud_run_connection_command_error"] = fmt.Sprintf("%s\n%s", err, out) - r.log.Warn("on_cloud_run_connection_command_error") + r.clients.IO.PrintError(ctx, "Error: %s\n%s", err, out) break } - r.log.Info("on_cloud_run_connection_command_output") - linkResponse = &LinkResponse{ EnvelopeID: msg.EnvelopeID, Payload: json.RawMessage(out), @@ -407,23 +404,19 @@ func (r *LocalServer) WatchManifest(ctx context.Context, auth types.SlackAuth, a return case event := <-w.Event: if isRemoteManifest { - r.log.Data["cloud_run_watch_manifest_change_skipped"] = event.Path - r.log.Info("on_cloud_run_watch_manifest_change_skipped_remote") + r.clients.IO.PrintInfo(ctx, false, "%s", style.Secondary(fmt.Sprintf("Manifest change detected: %s, skipped reinstalling app because manifest.source=remote", event.Path))) } else { - r.log.Data["cloud_run_watch_manifest_change"] = event.Path - r.log.Info("on_cloud_run_watch_manifest_change") + r.clients.IO.PrintInfo(ctx, false, "%s", style.Secondary(fmt.Sprintf("Manifest change detected: %s, reinstalling app...", event.Path))) // Reinstall the app when manifest changes if _, _, _, err := apps.InstallLocalApp(ctx, r.clients, "", auth, app); err != nil { - r.log.Data["cloud_run_watch_error"] = err.Error() - r.log.Warn("on_cloud_run_watch_error") + r.clients.IO.PrintError(ctx, "Error: %s", err) } else { - r.log.Info("on_cloud_run_watch_manifest_change_reinstalled") + r.clients.IO.PrintInfo(ctx, false, "%s", style.Secondary("App successfully reinstalled")) } } case err := <-w.Error: - r.log.Data["cloud_run_watch_error"] = err.Error() - r.log.Warn("on_cloud_run_watch_error") + r.clients.IO.PrintError(ctx, "Error: %s", err) case <-w.Closed: return } @@ -488,8 +481,7 @@ func (r *LocalServer) WatchApp(ctx context.Context) error { r.clients.IO.PrintDebug(ctx, "App file watcher context canceled, returning.") return case event := <-w.Event: - r.log.Data["cloud_run_watch_app_change"] = event.Path - r.log.Info("on_cloud_run_watch_app_change") + r.clients.IO.PrintInfo(ctx, false, "%s", style.Secondary(fmt.Sprintf("App change detected: %s, restarting server...", event.Path))) // Stop the previous process before starting a new one r.stopDelegateProcess(ctx) @@ -503,8 +495,7 @@ func (r *LocalServer) WatchApp(ctx context.Context) error { } }() case err := <-w.Error: - r.log.Data["cloud_run_watch_error"] = err.Error() - r.log.Warn("on_cloud_run_watch_error") + r.clients.IO.PrintError(ctx, "Error: %s", err) case <-w.Closed: return } diff --git a/internal/pkg/platform/localserver_test.go b/internal/pkg/platform/localserver_test.go index f5fc1418..e0b061b3 100644 --- a/internal/pkg/platform/localserver_test.go +++ b/internal/pkg/platform/localserver_test.go @@ -27,7 +27,6 @@ import ( "github.com/gorilla/websocket" "github.com/slackapi/slack-cli/internal/api" "github.com/slackapi/slack-cli/internal/hooks" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" @@ -140,12 +139,8 @@ func Test_LocalServer_Start(t *testing.T) { AppID: "A12345", TeamID: "justiceleague", } - log := logger.Logger{ - Data: map[string]interface{}{}, - } server := LocalServer{ clients, - &log, "ABC123", localContext, clients.SDKConfig, @@ -353,12 +348,8 @@ func Test_LocalServer_Listen(t *testing.T) { AppID: "A12345", TeamID: "justiceleague", } - log := logger.Logger{ - Data: map[string]interface{}{}, - } server := LocalServer{ clients, - &log, "ABC123", localContext, clients.SDKConfig, diff --git a/internal/pkg/platform/run.go b/internal/pkg/platform/run.go index 6a6d707c..d78a7d74 100644 --- a/internal/pkg/platform/run.go +++ b/internal/pkg/platform/run.go @@ -23,12 +23,12 @@ import ( "github.com/slackapi/slack-cli/cmd/feedback" "github.com/slackapi/slack-cli/cmd/triggers" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/logger" "github.com/slackapi/slack-cli/internal/pkg/apps" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/slacktrace" + "github.com/slackapi/slack-cli/internal/style" ) // RunArgs are the arguments passed into the Run function @@ -43,7 +43,7 @@ type RunArgs struct { } // Run locally runs your app. -func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, runArgs RunArgs) (*logger.LogEvent, types.InstallState, error) { +func Run(ctx context.Context, clients *shared.ClientFactory, runArgs RunArgs) (types.InstallState, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "cmd.create") defer span.Finish() @@ -54,9 +54,9 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, authSession, err := clients.API().ValidateSession(ctx, runArgs.Auth.Token) if err != nil { err = slackerror.Wrap(err, "No auth session found") - return nil, "", slackerror.Wrap(err, slackerror.ErrLocalAppRun) + return "", slackerror.Wrap(err, slackerror.ErrLocalAppRun) } - log.Data["teamName"] = *authSession.TeamName + teamName := *authSession.TeamName if authSession.UserID != nil { ctx = config.SetContextUserID(ctx, *authSession.UserID) @@ -73,17 +73,17 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, // For CLI Run command execute successfully if !clients.SDKConfig.Hooks.Start.IsAvailable() { var err = slackerror.New(slackerror.ErrSDKHookNotFound).WithMessage("The `start` script was not found") - return nil, "", err + return "", err } // Update local install installedApp, localInstallResult, installState, err := apps.InstallLocalApp(ctx, clients, runArgs.OrgGrantWorkspaceID, runArgs.Auth, runArgs.App) if err != nil { - return nil, "", slackerror.Wrap(err, slackerror.ErrLocalAppRun) + return "", slackerror.Wrap(err, slackerror.ErrLocalAppRun) } if installState == types.InstallRequestPending || installState == types.InstallRequestCancelled || installState == types.InstallRequestNotSent { - return log.SuccessEvent(), types.InstallSuccess, nil + return types.InstallSuccess, nil } if runArgs.ShowTriggers { @@ -93,17 +93,17 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, if strings.Contains(err.Error(), "workflow_not_found") { listErr := triggers.ListWorkflows(ctx, clients, runArgs.App, runArgs.Auth) if listErr != nil { - return nil, "", slackerror.Wrap(listErr, "Error listing workflows").WithRootCause(err) + return "", slackerror.Wrap(listErr, "Error listing workflows").WithRootCause(err) } } - return nil, "", err + return "", err } } // Gather environment variables from an environment file variables, err := clients.Config.GetDotEnvFileVariables() if err != nil { - return nil, "", slackerror.Wrap(err, slackerror.ErrLocalAppRun). + return "", slackerror.Wrap(err, slackerror.ErrLocalAppRun). WithMessage("Failed to read the local .env file") } @@ -123,7 +123,6 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, var server = LocalServer{ clients: clients, - log: log, token: localInstallResult.APIAccessTokens.AppLevel, localHostedContext: localHostedContext, cliConfig: cliConfig, @@ -141,7 +140,7 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, <-ctx.Done() clients.IO.PrintDebug(ctx, "Interrupt signal received in Run command, cleaning up and shutting down") if cleanup { - deleteAppOnTerminate(ctx, clients, runArgs.Auth, installedApp, log) + deleteAppOnTerminate(ctx, clients, runArgs.Auth, installedApp, teamName) } feedback.ShowFeedbackMessageOnTerminate(ctx, clients) // Notify Slack backend we are closing connection; this should trigger an echoing close message from Slack @@ -196,21 +195,22 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, if err := <-errChan; err != nil { switch slackerror.ToSlackError(err).Code { case slackerror.ErrLocalAppRunCleanExit: - return log.SuccessEvent(), types.InstallSuccess, nil + return types.InstallSuccess, nil case slackerror.ErrSDKHookInvocationFailed: - return nil, "", err + return "", err } - return nil, "", slackerror.Wrap(err, slackerror.ErrLocalAppRun) + return "", slackerror.Wrap(err, slackerror.ErrLocalAppRun) } - return log.SuccessEvent(), types.InstallSuccess, nil + return types.InstallSuccess, nil } -func deleteAppOnTerminate(ctx context.Context, clients *shared.ClientFactory, auth types.SlackAuth, app types.App, log *logger.Logger) { +func deleteAppOnTerminate(ctx context.Context, clients *shared.ClientFactory, auth types.SlackAuth, app types.App, teamName string) { clients.IO.PrintDebug(ctx, "Removing the local version of this app from the workspace") _, _, err := apps.Delete(ctx, clients, app.TeamDomain, app, auth) if err != nil { - log.Data["on_cleanup_app_install_error"] = err.Error() - log.Info("on_cleanup_app_install_failed") + clients.IO.PrintInfo(ctx, false, "%s", style.Secondary(fmt.Sprintf(`Cleaning up local app install for "%s" failed.`, teamName))) + clients.IO.PrintWarning(ctx, "Local app cleanup failed: %s", err) + } else { + clients.IO.PrintInfo(ctx, false, "%s", style.Secondary(fmt.Sprintf(`Cleaned up local app install for "%s".`, teamName))) } - log.Info("on_cleanup_app_install_done") } From 32a741d4b1d4871b6315f3a939d4e5ed41bbf6de Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Mon, 9 Mar 2026 17:16:49 -0700 Subject: [PATCH 5/5] fix: correct merge conflict resolutions for create.go and install.go --- cmd/platform/run.go | 1 - cmd/project/create.go | 5 +---- internal/pkg/apps/install.go | 32 ++++++++++++++++---------------- 3 files changed, 17 insertions(+), 21 deletions(-) diff --git a/cmd/platform/run.go b/cmd/platform/run.go index bcd8f12a..5e606ad2 100644 --- a/cmd/platform/run.go +++ b/cmd/platform/run.go @@ -149,4 +149,3 @@ func RunRunCommand(clients *shared.ClientFactory, cmd *cobra.Command, args []str return nil } - diff --git a/cmd/project/create.go b/cmd/project/create.go index 02c41d26..ab856cc2 100644 --- a/cmd/project/create.go +++ b/cmd/project/create.go @@ -42,8 +42,6 @@ var createSubdirFlag string // TODO - Find best practice, such as using an Interface and Struct to create a client var CreateFunc = create.Create -var appCreateSpinner *style.Spinner - // promptObject describes the Github app template type promptObject struct { Title string // "Reverse string" @@ -168,12 +166,11 @@ func runCreateCommand(clients *shared.ClientFactory, cmd *cobra.Command, args [] } clients.EventTracker.SetAppTemplate(template.GetTemplatePath()) - appCreateSpinner.Update("Creating app from template", "").Start() appDirPath, err := CreateFunc(ctx, clients, createArgs) if err != nil { return err } - appCreateSpinner.Update(style.Sectionf(style.TextSection{Emoji: "gear", Text: "Created project directory"}), "").Stop() + printCreateSuccess(ctx, clients, appDirPath) return nil } diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 731a067d..b27b7992 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -126,26 +126,26 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac start := time.Now() switch { case manifestUpdates: - clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + _, _ = clients.IO.WriteOut().Write([]byte("\n" + style.Sectionf(style.TextSection{ Emoji: "books", Text: "App Manifest", Secondary: []string{ fmt.Sprintf(`Updated app manifest for "%s" in "%s"`, slackManifest.DisplayInformation.Name, *authSession.TeamName), }, - })) + }))) clients.IO.PrintDebug(ctx, "updating app %s", app.AppID) _, err := apiInterface.UpdateApp(ctx, token, app.AppID, manifest, clients.Config.ForceFlag, true) if err != nil { return app, "", err } case manifestCreates: - clients.IO.PrintInfo(ctx, false, "%s", style.Sectionf(style.TextSection{ + _, _ = clients.IO.WriteOut().Write([]byte(style.Sectionf(style.TextSection{ Emoji: "books", Text: "App Manifest", Secondary: []string{ fmt.Sprintf(`Creating app manifest for "%s" in "%s"`, slackManifest.DisplayInformation.Name, *authSession.TeamName), }, - })) + }))) clients.IO.PrintDebug(ctx, "app not found so creating a new app") result, err := apiInterface.CreateApp(ctx, token, manifest, false) if err != nil { @@ -207,13 +207,13 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac outgoingDomains = *manifest.OutgoingDomains } - clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + _, _ = clients.IO.WriteOut().Write([]byte("\n" + style.Sectionf(style.TextSection{ Emoji: "house", Text: "App Install", Secondary: []string{ fmt.Sprintf(`Installing "%s" app to "%s"`, manifest.DisplayInformation.Name, *authSession.TeamName), }, - })) + }))) // Note - we use DeveloperAppInstall endpoint for both local (dev) runs // and hosted installs https://github.com/slackapi/slack-cli/pull/456#discussion_r830272175 @@ -245,9 +245,9 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac err = updateIcon(ctx, clients, iconPath, app.AppID, token) if err != nil { clients.IO.PrintDebug(ctx, "icon error: %s", err) - clients.IO.PrintInfo(ctx, false, "%s", style.SectionSecondaryf("Error updating app icon: %s", err)) + _, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Error updating app icon: %s", err))) } else { - clients.IO.PrintInfo(ctx, false, "%s", style.SectionSecondaryf("Updated app icon: %s", iconPath)) + _, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Updated app icon: %s", iconPath))) } // TODO: Optimization. // Save a md5 hash of the icon in environments.yaml @@ -261,7 +261,7 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac // update config with latest yaml hash // env.Hash = slackYaml.Hash - clients.IO.PrintInfo(ctx, false, "%s", style.SectionSecondaryf("Finished in %.1fs", time.Since(start).Seconds())) + _, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Finished in %.1fs", time.Since(start).Seconds()))) return app, types.InstallSuccess, nil } @@ -447,13 +447,13 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran start := time.Now() switch { case manifestUpdates: - clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + _, _ = clients.IO.WriteOut().Write([]byte("\n" + style.Sectionf(style.TextSection{ Emoji: "books", Text: "App Manifest", Secondary: []string{ fmt.Sprintf(`Updated app manifest for "%s" in "%s"`, slackManifest.DisplayInformation.Name, *authSession.TeamName), }, - })) + }))) clients.IO.PrintDebug(ctx, "updating app %s", app.AppID) _, err := apiInterface.UpdateApp(ctx, token, app.AppID, manifest, clients.Config.ForceFlag, true) if err != nil { @@ -461,13 +461,13 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran return app, api.DeveloperAppInstallResult{}, "", err } case manifestCreates: - clients.IO.PrintInfo(ctx, false, "%s", style.Sectionf(style.TextSection{ + _, _ = clients.IO.WriteOut().Write([]byte(style.Sectionf(style.TextSection{ Emoji: "books", Text: "App Manifest", Secondary: []string{ fmt.Sprintf(`Creating app manifest for "%s" in "%s"`, slackManifest.DisplayInformation.Name, *authSession.TeamName), }, - })) + }))) clients.IO.PrintDebug(ctx, "app not found so creating a new app") result, err := apiInterface.CreateApp(ctx, token, manifest, false) if err != nil { @@ -530,13 +530,13 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran outgoingDomains = *manifest.OutgoingDomains } - clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + _, _ = clients.IO.WriteOut().Write([]byte("\n" + style.Sectionf(style.TextSection{ Emoji: "house", Text: "App Install", Secondary: []string{ fmt.Sprintf(`Installing "%s" app to "%s"`, manifest.DisplayInformation.Name, *authSession.TeamName), }, - })) + }))) var installState types.InstallState result, installState, err := apiInterface.DeveloperAppInstall(ctx, clients.IO, token, app, botScopes, outgoingDomains, orgGrantWorkspaceID, clients.Config.AutoRequestAAAFlag) @@ -582,7 +582,7 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran // update config with latest yaml hash // env.Hash = slackYaml.Hash - clients.IO.PrintInfo(ctx, false, "%s", style.SectionSecondaryf("Finished in %.1fs", time.Since(start).Seconds())) + _, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Finished in %.1fs", time.Since(start).Seconds()))) return app, result, types.InstallSuccess, nil }