Skip to content

refactor(experiment): separate iostreams prompts from survey implementations #404

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
zimeg wants to merge 4 commits into
base: main
Choose a base branch
from
+710 −439
Open

refactor(experiment): separate iostreams prompts from survey implementations #404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87d927a
refactor: separate iostreams prompts from survey implementations
zimeg Mar 16, 2026
2f729ee
test: confirm coverage matches actual behaviors
zimeg Mar 16, 2026
e542ecf
Merge remote-tracking branch 'origin/main' into zimeg-refactor-iostre…
zimeg Mar 17, 2026
ff364f8
refactor: move DefaultSelectPromptConfig to prompts_mock.go
zimeg Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions cmd/datastore/count_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func TestCountCommand(t *testing.T) {
}, nil)
cm.IO.On("InputPrompt", mock.Anything, "Enter an expression", iostreams.InputPromptConfig{
Required: false,
}).Return("")
Copy link
Member Author

@zimeg zimeg Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧪 note: Lots of changes add the expected error value to mock prompts!

}).Return("", nil)
cm.API.On("AppsDatastoreCount", mock.Anything, mock.Anything, mock.Anything).
Return(types.AppDatastoreCountResult{Datastore: "numbers", Count: 12}, nil)
},
Expand Down Expand Up @@ -298,16 +298,16 @@ func TestCountCommand(t *testing.T) {
}, nil)
cm.IO.On("InputPrompt", mock.Anything, "Enter an expression", iostreams.InputPromptConfig{
Required: false,
}).Return("#n < :num AND #n <> :zero AND #prime = :bool")
}).Return("#n < :num AND #n <> :zero AND #prime = :bool", nil)
cm.IO.On("InputPrompt", mock.Anything, "Enter a value for ':num'", iostreams.InputPromptConfig{
Required: true,
}).Return("12")
}).Return("12", nil)
cm.IO.On("InputPrompt", mock.Anything, "Enter a value for ':zero'", iostreams.InputPromptConfig{
Required: true,
}).Return("0")
}).Return("0", nil)
cm.IO.On("InputPrompt", mock.Anything, "Enter a value for ':bool'", iostreams.InputPromptConfig{
Required: true,
}).Return("true")
}).Return("true", nil)
cm.API.On("AppsDatastoreCount", mock.Anything, mock.Anything, mock.Anything).
Return(types.AppDatastoreCountResult{Datastore: "numbers", Count: 6}, nil)
},
Expand Down
2 changes: 1 addition & 1 deletion cmd/datastore/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func TestDeleteCommand(t *testing.T) {
}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a task_id", iostreams.InputPromptConfig{
Required: true,
}).Return("1234")
}).Return("1234", nil)
},
Teardown: func() {
os.Args = os.Args[:len(os.Args)-1]
Expand Down
2 changes: 1 addition & 1 deletion cmd/datastore/get_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func TestGetCommand(t *testing.T) {
}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a task_id", iostreams.InputPromptConfig{
Required: true,
}).Return("1234")
}).Return("1234", nil)
},
Teardown: func() {
os.Args = os.Args[:len(os.Args)-1]
Expand Down
6 changes: 3 additions & 3 deletions cmd/datastore/put_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,13 @@ func TestPutCommand(t *testing.T) {
}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'task_id':", iostreams.InputPromptConfig{
Required: true,
}).Return("1234")
}).Return("1234", nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'task':", iostreams.InputPromptConfig{
Required: false,
}).Return("prompting for info")
}).Return("prompting for info", nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'status':", iostreams.InputPromptConfig{
Required: false,
}).Return("promptful")
}).Return("promptful", nil)
},
Teardown: func() {
os.Args = os.Args[:len(os.Args)-1]
Expand Down
10 changes: 5 additions & 5 deletions cmd/datastore/query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func TestQueryCommand(t *testing.T) {
}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter an expression", iostreams.InputPromptConfig{
Required: false,
}).Return("")
}).Return("", nil)
},
Teardown: func() {
os.Args = os.Args[:len(os.Args)-1]
Expand All @@ -269,7 +269,7 @@ func TestQueryCommand(t *testing.T) {
}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter an expression", iostreams.InputPromptConfig{
Required: false,
}).Return("#task_id < :num AND #task_id <> :zero AND #notes = :progress")
}).Return("#task_id < :num AND #task_id <> :zero AND #notes = :progress", nil)
clientsMock.IO.On("SelectPrompt", mock.Anything, "Select an attribute for '#notes'", mock.Anything, iostreams.MatchPromptConfig(iostreams.SelectPromptConfig{
Flag: clientsMock.Config.Flags.Lookup("attributes"),
})).Return(iostreams.SelectPromptResponse{
Expand All @@ -286,13 +286,13 @@ func TestQueryCommand(t *testing.T) {
}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for ':num'", iostreams.InputPromptConfig{
Required: true,
}).Return("3")
}).Return("3", nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for ':zero'", iostreams.InputPromptConfig{
Required: true,
}).Return("0")
}).Return("0", nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for ':progress'", iostreams.InputPromptConfig{
Required: true,
}).Return("wip")
}).Return("wip", nil)
},
Teardown: func() {
os.Args = os.Args[:len(os.Args)-1]
Expand Down
26 changes: 13 additions & 13 deletions cmd/datastore/update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@ func TestUpdateCommand(t *testing.T) {
}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'task_id':", iostreams.InputPromptConfig{
Required: true,
}).Return("8888")
clientsMock.IO.On("MultiSelectPrompt", mock.Anything, "Select fields to update", mock.Anything).Return([]string{})
}).Return("8888", nil)
clientsMock.IO.On("MultiSelectPrompt", mock.Anything, "Select fields to update", mock.Anything).Return([]string{}, nil)
},
Teardown: func() {
os.Args = os.Args[:len(os.Args)-1]
Expand All @@ -223,14 +223,14 @@ func TestUpdateCommand(t *testing.T) {
}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'task_id':", iostreams.InputPromptConfig{
Required: true,
}).Return("0000")
clientsMock.IO.On("MultiSelectPrompt", mock.Anything, "Select fields to update", mock.Anything).Return([]string{"status", "task"})
}).Return("0000", nil)
clientsMock.IO.On("MultiSelectPrompt", mock.Anything, "Select fields to update", mock.Anything).Return([]string{"status", "task"}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'status':", iostreams.InputPromptConfig{
Required: false,
}).Return("")
}).Return("", nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'task':", iostreams.InputPromptConfig{
Required: false,
}).Return("")
}).Return("", nil)
},
Teardown: func() {
os.Args = os.Args[:len(os.Args)-1]
Expand Down Expand Up @@ -259,11 +259,11 @@ func TestUpdateCommand(t *testing.T) {
}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'task_id':", iostreams.InputPromptConfig{
Required: true,
}).Return("1234")
clientsMock.IO.On("MultiSelectPrompt", mock.Anything, "Select fields to update", mock.Anything).Return([]string{"status"})
}).Return("1234", nil)
clientsMock.IO.On("MultiSelectPrompt", mock.Anything, "Select fields to update", mock.Anything).Return([]string{"status"}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'status':", iostreams.InputPromptConfig{
Required: false,
}).Return("getting closer")
}).Return("getting closer", nil)
},
Teardown: func() {
os.Args = os.Args[:len(os.Args)-1]
Expand Down Expand Up @@ -291,14 +291,14 @@ func TestUpdateCommand(t *testing.T) {
}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'task_id':", iostreams.InputPromptConfig{
Required: true,
}).Return("9999")
clientsMock.IO.On("MultiSelectPrompt", mock.Anything, "Select fields to update", mock.Anything).Return([]string{"task", "status"})
}).Return("9999", nil)
clientsMock.IO.On("MultiSelectPrompt", mock.Anything, "Select fields to update", mock.Anything).Return([]string{"task", "status"}, nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'task':", iostreams.InputPromptConfig{
Required: false,
}).Return("relax")
}).Return("relax", nil)
clientsMock.IO.On("InputPrompt", mock.Anything, "Enter a value for 'status':", iostreams.InputPromptConfig{
Required: false,
}).Return("always")
}).Return("always", nil)
},
Teardown: func() {
os.Args = os.Args[:len(os.Args)-1]
Expand Down
8 changes: 4 additions & 4 deletions cmd/externalauth/remove_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func TestExternalAuthRemoveCommand(t *testing.T) {
}}, nil)
clientsMock.API.On("AppsAuthExternalDelete", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
clientsMock.AddDefaultMocks()
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Are you sure you want to remove all tokens for this app relevant to the specified provider from your current team/org?", mock.Anything).Return(true)
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Are you sure you want to remove all tokens for this app relevant to the specified provider from your current team/org?", mock.Anything).Return(true, nil)
err := clients.AppClient().SaveDeployed(ctx, fakeApp)
require.NoError(t, err, "Cant write apps.json")
},
Expand All @@ -209,7 +209,7 @@ func TestExternalAuthRemoveCommand(t *testing.T) {
clientsMock.AddDefaultMocks()
err := clients.AppClient().SaveDeployed(ctx, fakeApp)
require.NoError(t, err, "Cant write apps.json")
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Are you sure you want to remove all tokens for this app from your current team/org?", mock.Anything).Return(true)
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Are you sure you want to remove all tokens for this app from your current team/org?", mock.Anything).Return(true, nil)
},
ExpectedAsserts: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock) {
clientsMock.API.AssertCalled(t, "AppsAuthExternalDelete", mock.Anything, mock.Anything, fakeAppID, "", "")
Expand All @@ -235,7 +235,7 @@ func TestExternalAuthRemoveCommand(t *testing.T) {
err := clients.AppClient().SaveDeployed(ctx, fakeApp)
require.NoError(t, err, "Cant write apps.json")

clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Are you sure you want to remove all tokens for this app relevant to the specified provider from your current team/org?", mock.Anything).Return(true)
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Are you sure you want to remove all tokens for this app relevant to the specified provider from your current team/org?", mock.Anything).Return(true, nil)
},
ExpectedAsserts: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock) {
clientsMock.API.AssertCalled(t, "AppsAuthExternalDelete", mock.Anything, mock.Anything, fakeAppID, "provider_a", "")
Expand All @@ -252,7 +252,7 @@ func TestExternalAuthRemoveCommand(t *testing.T) {
clientsMock.AddDefaultMocks()
err := clients.AppClient().SaveDeployed(ctx, fakeApp)
require.NoError(t, err, "Cant write apps.json")
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Are you sure you want to remove all tokens for this app from your current team/org?", mock.Anything).Return(true)
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Are you sure you want to remove all tokens for this app from your current team/org?", mock.Anything).Return(true, nil)
},
ExpectedAsserts: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock) {
clientsMock.API.AssertCalled(t, "AppsAuthExternalDelete", mock.Anything, mock.Anything, fakeAppID, "", "")
Expand Down
14 changes: 7 additions & 7 deletions cmd/feedback/feedback_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestFeedbackCommand(t *testing.T) {
scm.On("SetSurveyConfig", mock.Anything, mock.Anything, mock.Anything).Return(nil)
clientsMock.Config.SystemConfig = scm

clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Open in browser?", mock.Anything).Return(true)
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Open in browser?", mock.Anything).Return(true, nil)
clientsMock.Browser.On("OpenURL", "https://survey.com?project_id=projectID&system_id=systemID&utm_medium=cli&utm_source=cli").Return(nil)

clients := shared.NewClientFactory(clientsMock.MockClientFactory())
Expand Down Expand Up @@ -133,7 +133,7 @@ func TestFeedbackCommand(t *testing.T) {
Index: 2,
}, nil)

clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Open in browser?", mock.Anything).Return(true)
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Open in browser?", mock.Anything).Return(true, nil)
clientsMock.Browser.On("OpenURL", "https://survey.com?project_id=projectID&system_id=systemID&utm_medium=cli&utm_source=cli").Return(nil)

clients := shared.NewClientFactory(clientsMock.MockClientFactory())
Expand Down Expand Up @@ -223,19 +223,19 @@ func TestShowSurveyMessages(t *testing.T) {

// B
pcm.On("GetSurveyConfig", mock.Anything, "B").Return(config.SurveyConfig{}, slackerror.New(slackerror.ErrSurveyConfigNotFound)).Once()
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Would you like to take a minute to tell us about B?", mock.Anything).Return(true)
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Would you like to take a minute to tell us about B?", mock.Anything).Return(true, nil)
scm.On("GetSystemID", mock.Anything).Return("systemID", nil).Once()
pcm.On("GetProjectID", mock.Anything).Return("projectID", nil).Once()
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Open in browser?", mock.Anything).Return(true).Once()
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Open in browser?", mock.Anything).Return(true, nil).Once()
clientsMock.Browser.On("OpenURL", "https://B.com?project_id=projectID&system_id=systemID&utm_medium=cli&utm_source=cli").Return(nil).Once()
pcm.On("SetSurveyConfig", mock.Anything, "B", mock.Anything).Return(nil).Once()

// C
scm.On("GetSurveyConfig", mock.Anything, "C").Return(config.SurveyConfig{AskedAt: oneMonthAgoTimestamp}, nil).Once()
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Would you like to take a minute to tell us about C?", mock.Anything).Return(true)
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Would you like to take a minute to tell us about C?", mock.Anything).Return(true, nil)
scm.On("GetSystemID", mock.Anything).Return("systemID", nil).Once()
pcm.On("GetProjectID", mock.Anything).Return("projectID", nil).Once()
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Open in browser?", mock.Anything).Return(true).Once()
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Open in browser?", mock.Anything).Return(true, nil).Once()
clientsMock.Browser.On("OpenURL", "https://C.com?project_id=projectID&system_id=systemID&utm_medium=cli&utm_source=cli").Return(nil).Once()
scm.On("SetSurveyConfig", mock.Anything, "C", mock.Anything).Return(nil).Once()

Expand Down Expand Up @@ -315,5 +315,5 @@ func setupFeedbackCommandMocks(t *testing.T, ctx context.Context, cm *shared.Cli
pcm.On("GetProjectID", mock.Anything).Return("projectID", nil)
cm.Config.ProjectConfig = pcm

cm.IO.On("ConfirmPrompt", mock.Anything, "Open in browser?", mock.Anything).Return(false)
cm.IO.On("ConfirmPrompt", mock.Anything, "Open in browser?", mock.Anything).Return(false, nil)
}
2 changes: 1 addition & 1 deletion cmd/triggers/access_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func TestTriggersAccessCommand(t *testing.T) {
clientsMock.API.On("UserInfo", mock.Anything, mock.Anything, "collaborator_ID").
Return(&types.UserInfo{}, nil).Once()
// confirm to add app collaborators
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Include app collaborators?", mock.Anything).Return(true)
clientsMock.IO.On("ConfirmPrompt", mock.Anything, "Include app collaborators?", mock.Anything).Return(true, nil)
// set access type to named_entities
clientsMock.API.On("TriggerPermissionsSet", mock.Anything, mock.Anything, fakeTriggerID, "collaborator_ID", types.PermissionNamedEntities, "users").Maybe().
Return([]string{}, nil)
Expand Down
4 changes: 0 additions & 4 deletions internal/iostreams/charm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ func buildSelectForm(msg string, options []string, cfg SelectPromptConfig, selec
Options(opts...).
Value(selected)

if cfg.PageSize > 0 {
field.Height(cfg.PageSize + 2)
}
Comment on lines -88 to -90
Copy link
Member Author

@zimeg zimeg Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🪓 note: Alongside changes of #400 and #401 we're hoping to use default form heights while the experiment for charm prompts remains active.


return huh.NewForm(huh.NewGroup(field)).WithTheme(style.ThemeSlack())
}

Expand Down
Loading
Loading