Skip to content

Commit 83f47e1

Browse files
mwbrooksmwbrooks
committed
refactor: remove logger from activity, validate, datastore, auth list, delete, and uninstall
1 parent c449532 commit 83f47e1

52 files changed

Lines changed: 201 additions & 593 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎cmd/app/delete.go‎

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"github.com/slackapi/slack-cli/internal/cmdutil"
2222
"github.com/slackapi/slack-cli/internal/iostreams"
23-
"github.com/slackapi/slack-cli/internal/logger"
2423
"github.com/slackapi/slack-cli/internal/pkg/apps"
2524
"github.com/slackapi/slack-cli/internal/prompts"
2625
"github.com/slackapi/slack-cli/internal/shared"
@@ -99,30 +98,14 @@ func RunDeleteCommand(ctx context.Context, clients *shared.ClientFactory, cmd *c
9998
}
10099
}
101100

102-
// Set up event logger and execute the command
103-
log := newDeleteLogger(clients, cmd, team)
104-
log.Data["appID"] = selection.App.AppID
105-
env, err := apps.Delete(ctx, clients, log, team, selection.App, selection.Auth)
106-
107-
return env, err
108-
}
101+
// Execute the command
102+
env, teamName, err := apps.Delete(ctx, clients, team, selection.App, selection.Auth)
103+
if err != nil {
104+
return env, err
105+
}
106+
printDeleteApp(ctx, clients, selection.App.AppID, teamName)
109107

110-
// newDeleteLogger creates a logger instance to receive event notifications
111-
func newDeleteLogger(clients *shared.ClientFactory, cmd *cobra.Command, envName string) *logger.Logger {
112-
ctx := cmd.Context()
113-
return logger.New(
114-
// OnEvent
115-
func(event *logger.LogEvent) {
116-
appID := event.DataToString("appID")
117-
teamName := event.DataToString("teamName")
118-
switch event.Name {
119-
case "on_apps_delete_app_success":
120-
printDeleteApp(ctx, clients, appID, teamName)
121-
default:
122-
// Ignore the event
123-
}
124-
},
125-
)
108+
return env, nil
126109
}
127110

128111
func confirmDeletion(ctx context.Context, IO iostreams.IOStreamer, app prompts.SelectedApp) (bool, error) {

‎cmd/app/uninstall.go‎

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"github.com/slackapi/slack-cli/internal/cmdutil"
2222
"github.com/slackapi/slack-cli/internal/iostreams"
23-
"github.com/slackapi/slack-cli/internal/logger"
2423
"github.com/slackapi/slack-cli/internal/pkg/apps"
2524
"github.com/slackapi/slack-cli/internal/prompts"
2625
"github.com/slackapi/slack-cli/internal/shared"
@@ -94,30 +93,14 @@ func RunUninstallCommand(ctx context.Context, clients *shared.ClientFactory, cmd
9493
}
9594
}
9695

97-
// Set up event logger and execute the command
98-
log := newUninstallLogger(clients, cmd, teamDomain)
99-
log.Data["appID"] = selection.App.AppID
100-
env, err := apps.Uninstall(ctx, clients, log, teamDomain, selection.App, selection.Auth)
101-
102-
return env, err
103-
}
96+
// Execute the command
97+
env, teamName, err := apps.Uninstall(ctx, clients, teamDomain, selection.App, selection.Auth)
98+
if err != nil {
99+
return env, err
100+
}
101+
printUninstallApp(ctx, clients, selection.App.AppID, teamName)
104102

105-
// newUninstallLogger creates a logger instance to receive event notifications
106-
func newUninstallLogger(clients *shared.ClientFactory, cmd *cobra.Command, envName string) *logger.Logger {
107-
ctx := cmd.Context()
108-
return logger.New(
109-
// OnEvent
110-
func(event *logger.LogEvent) {
111-
appID := event.DataToString("appID")
112-
teamName := event.DataToString("teamName")
113-
switch event.Name {
114-
case "on_apps_uninstall_app_success":
115-
printUninstallApp(ctx, clients, appID, teamName)
116-
default:
117-
// Ignore the event
118-
}
119-
},
120-
)
103+
return env, nil
121104
}
122105

123106
func confirmUninstall(ctx context.Context, IO iostreams.IOStreamer, cmd *cobra.Command, selection prompts.SelectedApp) (bool, error) {

‎cmd/auth/auth_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"testing"
2020

2121
"github.com/slackapi/slack-cli/internal/hooks"
22-
"github.com/slackapi/slack-cli/internal/logger"
2322
"github.com/slackapi/slack-cli/internal/shared"
2423
"github.com/slackapi/slack-cli/internal/shared/types"
2524
"github.com/slackapi/slack-cli/internal/slackcontext"
@@ -32,7 +31,7 @@ type listMockObject struct {
3231
mock.Mock
3332
}
3433

35-
func (m *listMockObject) MockListFunction(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger) ([]types.SlackAuth, error) {
34+
func (m *listMockObject) MockListFunction(ctx context.Context, clients *shared.ClientFactory) ([]types.SlackAuth, error) {
3635
args := m.Called()
3736
return args.Get(0).([]types.SlackAuth), args.Error(1)
3837
}

‎cmd/auth/list.go‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"fmt"
1919

2020
"github.com/slackapi/slack-cli/internal/iostreams"
21-
"github.com/slackapi/slack-cli/internal/logger"
2221
"github.com/slackapi/slack-cli/internal/pkg/auth"
2322
"github.com/slackapi/slack-cli/internal/shared"
2423
"github.com/slackapi/slack-cli/internal/shared/types"
@@ -52,34 +51,15 @@ func NewListCommand(clients *shared.ClientFactory) *cobra.Command {
5251
// runListCommand will execute the list command
5352
func runListCommand(cmd *cobra.Command, clients *shared.ClientFactory) error {
5453
ctx := cmd.Context()
55-
log := newListLogger(cmd, clients.IO)
56-
userAuthList, err := listFunc(ctx, clients, log)
54+
userAuthList, err := listFunc(ctx, clients)
5755
if err != nil {
5856
return err
5957
}
58+
printAuthList(cmd, clients.IO, userAuthList)
6059
printAuthListSuccess(cmd, clients.IO, userAuthList)
6160
return nil
6261
}
6362

64-
// newListLogger creates a logger instance to receive event notifications
65-
func newListLogger(cmd *cobra.Command, IO iostreams.IOStreamer) *logger.Logger {
66-
return logger.New(
67-
// OnEvent
68-
func(event *logger.LogEvent) {
69-
switch event.Name {
70-
case "on_auth_list":
71-
userAuthList := []types.SlackAuth{}
72-
if event.Data["userAuthList"] != nil {
73-
userAuthList = event.Data["userAuthList"].([]types.SlackAuth)
74-
}
75-
printAuthList(cmd, IO, userAuthList)
76-
default:
77-
// Ignore the event
78-
}
79-
},
80-
)
81-
}
82-
8363
// printAuthList will display a list of all authorizations available and highlight the default.
8464
// The output will be a list formatted as:
8565
//

‎cmd/auth/list_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"context"
1919
"testing"
2020

21-
"github.com/slackapi/slack-cli/internal/logger"
2221
"github.com/slackapi/slack-cli/internal/shared"
2322
"github.com/slackapi/slack-cli/internal/shared/types"
2423
"github.com/slackapi/slack-cli/internal/slackcontext"
@@ -32,7 +31,7 @@ type ListPkgMock struct {
3231
mock.Mock
3332
}
3433

35-
func (m *ListPkgMock) List(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger) ([]types.SlackAuth, error) {
34+
func (m *ListPkgMock) List(ctx context.Context, clients *shared.ClientFactory) ([]types.SlackAuth, error) {
3635
m.Called()
3736
return []types.SlackAuth{}, nil
3837
}

‎cmd/datastore/bulk_delete.go‎

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"github.com/slackapi/slack-cli/internal/cmdutil"
2222
"github.com/slackapi/slack-cli/internal/config"
2323
"github.com/slackapi/slack-cli/internal/goutils"
24-
"github.com/slackapi/slack-cli/internal/logger"
2524
"github.com/slackapi/slack-cli/internal/pkg/datastore"
2625
"github.com/slackapi/slack-cli/internal/prompts"
2726
"github.com/slackapi/slack-cli/internal/shared"
@@ -88,12 +87,12 @@ func NewBulkDeleteCommand(clients *shared.ClientFactory) *cobra.Command {
8887
}
8988

9089
// Perform the delete
91-
log := newBulkDeleteLogger(clients, cmd)
92-
event, err := BulkDelete(ctx, clients, log, query)
90+
result, err := BulkDelete(ctx, clients, query)
9391
if err != nil {
9492
return err
9593
}
96-
printDatastoreBulkDeleteSuccess(cmd, event)
94+
_ = printBulkDeleteResult(clients, cmd, result)
95+
printDatastoreBulkDeleteSuccess(cmd)
9796
return nil
9897
},
9998
}
@@ -118,26 +117,6 @@ func preRunBulkDeleteCommandFunc(ctx context.Context, clients *shared.ClientFact
118117
return cmdutil.IsSlackHostedProject(ctx, clients)
119118
}
120119

121-
func newBulkDeleteLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger {
122-
return logger.New(
123-
// OnEvent
124-
func(event *logger.LogEvent) {
125-
switch event.Name {
126-
case "on_bulk_delete_result":
127-
deleteResult := types.AppDatastoreBulkDeleteResult{}
128-
if event.Data["bulkDeleteResult"] != nil {
129-
deleteResult = event.Data["bulkDeleteResult"].(types.AppDatastoreBulkDeleteResult)
130-
}
131-
if cmd != nil {
132-
_ = printBulkDeleteResult(clients, cmd, deleteResult)
133-
}
134-
default:
135-
// Ignore the event
136-
}
137-
},
138-
)
139-
}
140-
141120
func printBulkDeleteResult(clients *shared.ClientFactory, cmd *cobra.Command, deleteResult types.AppDatastoreBulkDeleteResult) error {
142121
var datastore = deleteResult.Datastore
143122
cmd.Printf(
@@ -166,7 +145,7 @@ func printBulkDeleteResult(clients *shared.ClientFactory, cmd *cobra.Command, de
166145
return nil
167146
}
168147

169-
func printDatastoreBulkDeleteSuccess(cmd *cobra.Command, event *logger.LogEvent) {
148+
func printDatastoreBulkDeleteSuccess(cmd *cobra.Command) {
170149
commandText := style.Commandf("datastore query <expression>", true)
171150
if cmd != nil {
172151
cmd.Printf(

‎cmd/datastore/bulk_delete_test.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222

2323
"github.com/slackapi/slack-cli/internal/app"
2424
"github.com/slackapi/slack-cli/internal/config"
25-
"github.com/slackapi/slack-cli/internal/logger"
2625
"github.com/slackapi/slack-cli/internal/shared"
2726
"github.com/slackapi/slack-cli/internal/shared/types"
2827
"github.com/slackapi/slack-cli/internal/slackcontext"
@@ -36,11 +35,9 @@ type BulkDeletePkgMock struct {
3635
mock.Mock
3736
}
3837

39-
func (m *BulkDeletePkgMock) BulkDelete(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, query types.AppDatastoreBulkDelete) (*logger.LogEvent, error) {
40-
m.Called(ctx, clients, log, query)
41-
log.Data["bulkDeleteResult"] = types.AppDatastoreBulkDeleteResult{}
42-
log.Log("info", "on_bulk_delete_result")
43-
return log.SuccessEvent(), nil
38+
func (m *BulkDeletePkgMock) BulkDelete(ctx context.Context, clients *shared.ClientFactory, query types.AppDatastoreBulkDelete) (types.AppDatastoreBulkDeleteResult, error) {
39+
m.Called(ctx, clients, query)
40+
return types.AppDatastoreBulkDeleteResult{}, nil
4441
}
4542

4643
func TestBulkDeleteCommandPreRun(t *testing.T) {
@@ -185,7 +182,7 @@ func TestBulkDeleteCommand(t *testing.T) {
185182
// Create mocked command
186183
bulkDeleteMock := new(BulkDeletePkgMock)
187184
BulkDelete = bulkDeleteMock.BulkDelete
188-
bulkDeleteMock.On("BulkDelete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
185+
bulkDeleteMock.On("BulkDelete", mock.Anything, mock.Anything, mock.Anything).Return(types.AppDatastoreBulkDeleteResult{}, nil)
189186

190187
cmd := NewBulkDeleteCommand(clients)
191188
// 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) {
198195
// Create mocked command
199196
err := cmd.ExecuteContext(ctx)
200197
if assert.NoError(t, err) {
201-
bulkDeleteMock.AssertCalled(t, "BulkDelete", mock.Anything, mock.Anything, mock.Anything, tc.Query)
198+
bulkDeleteMock.AssertCalled(t, "BulkDelete", mock.Anything, mock.Anything, tc.Query)
202199
}
203200

204201
// Cleanup when done

‎cmd/datastore/bulk_get.go‎

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"github.com/slackapi/slack-cli/internal/cmdutil"
2323
"github.com/slackapi/slack-cli/internal/config"
2424
"github.com/slackapi/slack-cli/internal/goutils"
25-
"github.com/slackapi/slack-cli/internal/logger"
2625
"github.com/slackapi/slack-cli/internal/pkg/datastore"
2726
"github.com/slackapi/slack-cli/internal/prompts"
2827
"github.com/slackapi/slack-cli/internal/shared"
@@ -90,12 +89,12 @@ func NewBulkGetCommand(clients *shared.ClientFactory) *cobra.Command {
9089
}
9190

9291
// Perform the get
93-
log := newBulkGetLogger(clients, cmd, query)
94-
event, err := BulkGet(ctx, clients, log, query)
92+
result, err := BulkGet(ctx, clients, query)
9593
if err != nil {
9694
return err
9795
}
98-
printDatastoreBulkGetSuccess(cmd, event)
96+
_ = printBulkGetResult(clients, cmd, query, result)
97+
printDatastoreBulkGetSuccess(cmd)
9998
return nil
10099
},
101100
}
@@ -121,27 +120,6 @@ func preRunBulkGetCommandFunc(ctx context.Context, clients *shared.ClientFactory
121120
return cmdutil.IsSlackHostedProject(ctx, clients)
122121
}
123122

124-
func newBulkGetLogger(clients *shared.ClientFactory, cmd *cobra.Command, request types.AppDatastoreBulkGet) *logger.Logger {
125-
return logger.New(
126-
// OnEvent
127-
func(event *logger.LogEvent) {
128-
switch event.Name {
129-
case "on_bulk_get_result":
130-
getResult := types.AppDatastoreBulkGetResult{}
131-
if event.Data["bulkGetResult"] != nil {
132-
getResult = event.Data["bulkGetResult"].(types.AppDatastoreBulkGetResult)
133-
}
134-
if cmd != nil {
135-
// TODO: this can raise an error on indentation failures, but not sure how to handle that using logger
136-
_ = printBulkGetResult(clients, cmd, request, getResult)
137-
}
138-
default:
139-
// Ignore the event
140-
}
141-
},
142-
)
143-
}
144-
145123
func printBulkGetResult(clients *shared.ClientFactory, cmd *cobra.Command, request types.AppDatastoreBulkGet, getResult types.AppDatastoreBulkGetResult) error {
146124
var datastore = getResult.Datastore
147125
var items = getResult.Items
@@ -189,5 +167,5 @@ func printBulkGetResult(clients *shared.ClientFactory, cmd *cobra.Command, reque
189167
return nil
190168
}
191169

192-
func printDatastoreBulkGetSuccess(cmd *cobra.Command, event *logger.LogEvent) {
170+
func printDatastoreBulkGetSuccess(cmd *cobra.Command) {
193171
}

‎cmd/datastore/bulk_get_test.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222

2323
"github.com/slackapi/slack-cli/internal/app"
2424
"github.com/slackapi/slack-cli/internal/config"
25-
"github.com/slackapi/slack-cli/internal/logger"
2625
"github.com/slackapi/slack-cli/internal/shared"
2726
"github.com/slackapi/slack-cli/internal/shared/types"
2827
"github.com/slackapi/slack-cli/internal/slackcontext"
@@ -36,11 +35,9 @@ type BulkGetPkgMock struct {
3635
mock.Mock
3736
}
3837

39-
func (m *BulkGetPkgMock) BulkGet(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, query types.AppDatastoreBulkGet) (*logger.LogEvent, error) {
40-
m.Called(ctx, clients, log, query)
41-
log.Data["bulkGetResult"] = types.AppDatastoreBulkGetResult{}
42-
log.Log("info", "on_bulk_get_result")
43-
return log.SuccessEvent(), nil
38+
func (m *BulkGetPkgMock) BulkGet(ctx context.Context, clients *shared.ClientFactory, query types.AppDatastoreBulkGet) (types.AppDatastoreBulkGetResult, error) {
39+
m.Called(ctx, clients, query)
40+
return types.AppDatastoreBulkGetResult{}, nil
4441
}
4542

4643
func TestBulkGetCommandPreRun(t *testing.T) {
@@ -185,7 +182,7 @@ func TestBulkGetCommand(t *testing.T) {
185182
// Prepare mocked command
186183
bulkGetMock := new(BulkGetPkgMock)
187184
BulkGet = bulkGetMock.BulkGet
188-
bulkGetMock.On("BulkGet", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
185+
bulkGetMock.On("BulkGet", mock.Anything, mock.Anything, mock.Anything).Return(types.AppDatastoreBulkGetResult{}, nil)
189186

190187
cmd := NewBulkGetCommand(clients)
191188
// 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) {
198195
// Perform test
199196
err := cmd.ExecuteContext(ctx)
200197
if assert.NoError(t, err) {
201-
bulkGetMock.AssertCalled(t, "BulkGet", mock.Anything, mock.Anything, mock.Anything, tc.Query)
198+
bulkGetMock.AssertCalled(t, "BulkGet", mock.Anything, mock.Anything, tc.Query)
202199
}
203200

204201
// Cleanup when done

0 commit comments

Comments
 (0)