Skip to content

Commit e0a4db3

Browse files
committed
address review feedback on compositions interactive
- search: reject a positional query or --hits-per-page/--page/--filters when --interactive is set, instead of silently ignoring them - drop the redundant nil-Prompter fallback in upsert, rules upsert, search, and sorting-strategy (factory.New always sets it; Builder errors on nil anyway)
1 parent 020ce0a commit e0a4db3

5 files changed

Lines changed: 20 additions & 20 deletions

File tree

pkg/cmd/compositions/rules/upsert/upsert.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,7 @@ func buildRule(opts *UpsertOptions) (algoliaComposition.CompositionRule, error)
8585

8686
if opts.Interactive {
8787
rule.ObjectID = opts.ObjectID
88-
prompter := opts.Prompter
89-
if prompter == nil {
90-
prompter = interactive.NewSurveyPrompter(opts.IO)
91-
}
92-
if err := (&interactive.Builder{Prompter: prompter}).Build(&rule); err != nil {
88+
if err := (&interactive.Builder{Prompter: opts.Prompter}).Build(&rule); err != nil {
9389
return rule, fmt.Errorf("building rule: %w", err)
9490
}
9591
return rule, nil

pkg/cmd/compositions/search/search.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ func NewSearchCmd(f *cmdutil.Factory) *cobra.Command {
6868
if !opts.IO.CanPrompt() {
6969
return cmdutil.FlagErrorf("`--interactive` requires a terminal")
7070
}
71+
if len(args) > 1 || cmd.Flags().Changed("hits-per-page") || cmd.Flags().Changed("page") || cmd.Flags().Changed("filters") {
72+
return cmdutil.FlagErrorf("`--interactive` builds the whole request; omit the query argument and the --hits-per-page/--page/--filters flags")
73+
}
7174
return runSearchCmd(opts)
7275
}
7376

@@ -99,11 +102,7 @@ func NewSearchCmd(f *cmdutil.Factory) *cobra.Command {
99102
func buildRequestBody(opts *SearchOptions) (*algoliaComposition.RequestBody, error) {
100103
if opts.Interactive {
101104
var body algoliaComposition.RequestBody
102-
prompter := opts.Prompter
103-
if prompter == nil {
104-
prompter = interactive.NewSurveyPrompter(opts.IO)
105-
}
106-
if err := (&interactive.Builder{Prompter: prompter}).Build(&body); err != nil {
105+
if err := (&interactive.Builder{Prompter: opts.Prompter}).Build(&body); err != nil {
107106
return nil, fmt.Errorf("building search request: %w", err)
108107
}
109108
return &body, nil

pkg/cmd/compositions/search/search_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,16 @@ func TestSearchComposition_InteractiveNoTTY(t *testing.T) {
8989
require.Error(t, err)
9090
assert.Contains(t, err.Error(), "requires a terminal")
9191
}
92+
93+
func TestSearchComposition_InteractiveRejectsQueryAndFlags(t *testing.T) {
94+
// --interactive builds the whole request; a positional query or the flags
95+
// would be silently ignored, so they are rejected.
96+
for _, cli := range []string{`my-comp "shirt" --interactive`, "my-comp --interactive --hits-per-page 5"} {
97+
r := &httpmock.Registry{}
98+
f, out := test.NewFactory(true, r, nil, "")
99+
cmd := compsearch.NewSearchCmd(f)
100+
_, err := test.Execute(cmd, cli, out)
101+
require.Error(t, err)
102+
assert.Contains(t, err.Error(), "builds the whole request")
103+
}
104+
}

pkg/cmd/compositions/sortingstrategy/sortingstrategy.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,7 @@ func buildStrategy(opts *Options) (map[string]string, error) {
9090
var doc struct {
9191
SortingStrategy map[string]string `json:"sortingStrategy"`
9292
}
93-
prompter := opts.Prompter
94-
if prompter == nil {
95-
prompter = interactive.NewSurveyPrompter(opts.IO)
96-
}
97-
if err := (&interactive.Builder{Prompter: prompter}).Build(&doc); err != nil {
93+
if err := (&interactive.Builder{Prompter: opts.Prompter}).Build(&doc); err != nil {
9894
return nil, fmt.Errorf("building sorting strategy: %w", err)
9995
}
10096
return doc.SortingStrategy, nil

pkg/cmd/compositions/upsert/upsert.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,7 @@ func buildComposition(opts *UpsertOptions) (algoliaComposition.Composition, erro
8383

8484
if opts.Interactive {
8585
comp.ObjectID = opts.CompositionID
86-
prompter := opts.Prompter
87-
if prompter == nil {
88-
prompter = interactive.NewSurveyPrompter(opts.IO)
89-
}
90-
builder := &interactive.Builder{Prompter: prompter}
86+
builder := &interactive.Builder{Prompter: opts.Prompter}
9187
if err := builder.Build(&comp); err != nil {
9288
return comp, fmt.Errorf("building composition: %w", err)
9389
}

0 commit comments

Comments
 (0)