chore: introduce flags helper for consistency

rhamzeh · rhamzeh · commit 4ce74a1a59ea · 2025-07-07T10:48:21.000-04:00
diff --git a/cmd/model/get.go b/cmd/model/get.go
@@ -24,6 +24,7 @@ import (
 
 	"github.com/openfga/cli/internal/authorizationmodel"
 	"github.com/openfga/cli/internal/cmdutils"
+	"github.com/openfga/cli/internal/flags"
 	"github.com/openfga/cli/internal/output"
 )
 
@@ -77,8 +78,8 @@ func init() {
 	getCmd.Flags().StringArray("field", []string{"model"}, "Fields to display, choices are: id, created_at and model") //nolint:lll
 	getCmd.Flags().Var(&getOutputFormat, "format", `Authorization model output format. Can be "fga" or "json"`)
 
-	if err := getCmd.MarkFlagRequired("store-id"); err != nil {
-		fmt.Printf("error setting flag as required - %v: %v\n", "cmd/models/get", err)
+	if err := flags.SetFlagRequired(getCmd, "store-id", "cmd/models/get", false); err != nil {
+		fmt.Print(err)
 		os.Exit(1)
 	}
 }
diff --git a/cmd/model/list.go b/cmd/model/list.go
@@ -27,6 +27,7 @@ import (
 
 	"github.com/openfga/cli/internal/authorizationmodel"
 	"github.com/openfga/cli/internal/cmdutils"
+	"github.com/openfga/cli/internal/flags"
 	"github.com/openfga/cli/internal/output"
 )
 
@@ -112,8 +113,8 @@ func init() {
 	listCmd.Flags().String("store-id", "", "Store ID")
 	listCmd.Flags().StringArray("field", []string{"id", "created_at"}, "Fields to display, choices are: id, created_at and model") //nolint:lll
 
-	if err := listCmd.MarkFlagRequired("store-id"); err != nil {
-		fmt.Printf("error setting flag as required - %v: %v\n", "cmd/models/list", err)
+	if err := flags.SetFlagRequired(listCmd, "store-id", "cmd/models/list", false); err != nil {
+		fmt.Print(err)
 		os.Exit(1)
 	}
 }
diff --git a/cmd/model/test.go b/cmd/model/test.go
@@ -24,6 +24,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/openfga/cli/internal/cmdutils"
+	"github.com/openfga/cli/internal/flags"
 	"github.com/openfga/cli/internal/output"
 	"github.com/openfga/cli/internal/storetest"
 )
@@ -99,8 +100,8 @@ func init() {
 	testCmd.Flags().Bool("verbose", false, "Print verbose JSON output")
 	testCmd.Flags().Bool("suppress-summary", false, "Suppress the plain text summary output")
 
-	if err := testCmd.MarkFlagRequired("tests"); err != nil {
-		fmt.Printf("error setting flag as required - %v: %v\n", "cmd/models/test", err)
+	if err := flags.SetFlagRequired(testCmd, "tests", "cmd/models/test", false); err != nil {
+		fmt.Print(err)
 		os.Exit(1)
 	}
 }
diff --git a/cmd/model/write.go b/cmd/model/write.go
@@ -27,6 +27,7 @@ import (
 
 	"github.com/openfga/cli/internal/authorizationmodel"
 	"github.com/openfga/cli/internal/cmdutils"
+	"github.com/openfga/cli/internal/flags"
 	"github.com/openfga/cli/internal/output"
 	"github.com/openfga/cli/internal/utils"
 )
@@ -106,8 +107,8 @@ func init() {
 	writeCmd.Flags().String("file", "", "File Name. The file should have the model in the JSON or DSL format")
 	writeCmd.Flags().Var(&writeInputFormat, "format", `Authorization model input format. Can be "fga", "json", or "modular"`) //nolint:lll
 
-	if err := writeCmd.MarkFlagRequired("store-id"); err != nil {
-		fmt.Printf("error setting flag as required - %v: %v\n", "cmd/models/write", err)
+	if err := flags.SetFlagRequired(writeCmd, "tests", "cmd/models/write", false); err != nil {
+		fmt.Print(err)
 		os.Exit(1)
 	}
 }
diff --git a/cmd/query/list-users.go b/cmd/query/list-users.go
@@ -27,6 +27,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/openfga/cli/internal/cmdutils"
+	"github.com/openfga/cli/internal/flags"
 	"github.com/openfga/cli/internal/output"
 )
 
@@ -138,18 +139,8 @@ func init() {
 	listUsersCmd.Flags().String("relation", "", "Relation to evaluate on")
 	listUsersCmd.Flags().String("user-filter", "", "Filter the responses can be in the formats <type> (to filter objects and typed public bound access) or <type>#<relation> (to filter usersets)") //nolint:lll
 
-	if err := listUsersCmd.MarkFlagRequired("object"); err != nil {
-		fmt.Printf("error setting flag as required - %v: %v\n", "cmd/query/list-users", err)
-		os.Exit(1)
-	}
-
-	if err := listUsersCmd.MarkFlagRequired("relation"); err != nil {
-		fmt.Printf("error setting flag as required - %v: %v\n", "cmd/query/list-users", err)
-		os.Exit(1)
-	}
-
-	if err := listUsersCmd.MarkFlagRequired("user-filter"); err != nil {
-		fmt.Printf("error setting flag as required - %v: %v\n", "cmd/query/list-users", err)
+	if err := flags.SetFlagsRequired(listUsersCmd, []string{"object", "relation", "user-filter"}, "cmd/query/list-users", false); err != nil {
+		fmt.Print(err)
 		os.Exit(1)
 	}
 }
diff --git a/cmd/store/delete.go b/cmd/store/delete.go
@@ -25,6 +25,7 @@ import (
 
 	"github.com/openfga/cli/internal/cmdutils"
 	"github.com/openfga/cli/internal/confirmation"
+	"github.com/openfga/cli/internal/flags"
 	"github.com/openfga/cli/internal/output"
 )
 
@@ -72,8 +73,7 @@ func init() {
 	deleteCmd.Flags().String("store-id", "", "Store ID")
 	deleteCmd.Flags().Bool("force", false, "Force delete without confirmation")
 
-	err := deleteCmd.MarkFlagRequired("store-id")
-	if err != nil {
+	if err := flags.SetFlagRequired(deleteCmd, "store-id", "cmd/store/delete", false); err != nil {
 		fmt.Print(err)
 		os.Exit(1)
 	}
diff --git a/cmd/store/export.go b/cmd/store/export.go
@@ -30,6 +30,7 @@ import (
 	"github.com/openfga/cli/internal/cmdutils"
 	"github.com/openfga/cli/internal/confirmation"
 	"github.com/openfga/cli/internal/fga"
+	"github.com/openfga/cli/internal/flags"
 	"github.com/openfga/cli/internal/output"
 	"github.com/openfga/cli/internal/storetest"
 	"github.com/openfga/cli/internal/tuple"
@@ -193,8 +194,7 @@ func init() {
 	exportCmd.Flags().String("model-id", "", "Authorization Model ID")
 	exportCmd.Flags().Uint("max-tuples", defaultMaxTupleCount, "max number of tuples to return in the output")
 
-	err := exportCmd.MarkFlagRequired("store-id")
-	if err != nil {
+	if err := flags.SetFlagRequired(exportCmd, "store-id", "cmd/store/export", false); err != nil {
 		fmt.Print(err)
 		os.Exit(1)
 	}
diff --git a/cmd/store/get.go b/cmd/store/get.go
@@ -26,6 +26,7 @@ import (
 
 	"github.com/openfga/cli/internal/cmdutils"
 	"github.com/openfga/cli/internal/fga"
+	"github.com/openfga/cli/internal/flags"
 	"github.com/openfga/cli/internal/output"
 )
 
@@ -65,8 +66,7 @@ var getCmd = &cobra.Command{
 func init() {
 	getCmd.Flags().String("store-id", "", "Store ID")
 
-	err := getCmd.MarkFlagRequired("store-id")
-	if err != nil {
+	if err := flags.SetFlagRequired(getCmd, "store-id", "cmd/store/get", false); err != nil {
 		fmt.Print(err)
 		os.Exit(1)
 	}
diff --git a/cmd/store/import.go b/cmd/store/import.go
@@ -34,6 +34,7 @@ import (
 	"github.com/openfga/cli/internal/authorizationmodel"
 	"github.com/openfga/cli/internal/cmdutils"
 	"github.com/openfga/cli/internal/fga"
+	"github.com/openfga/cli/internal/flags"
 	"github.com/openfga/cli/internal/output"
 	"github.com/openfga/cli/internal/storetest"
 	"github.com/openfga/cli/internal/tuple"
@@ -339,8 +340,8 @@ func init() {
 	importCmd.Flags().Int("max-tuples-per-write", tuple.MaxTuplesPerWrite, "Max tuples per write chunk.")
 	importCmd.Flags().Int("max-parallel-requests", tuple.MaxParallelRequests, "Max number of requests to issue to the server in parallel.") //nolint:lll
 
-	if err := importCmd.MarkFlagRequired("file"); err != nil {
-		fmt.Printf("error setting flag as required - %v: %v\n", "cmd/models/write", err)
+	if err := flags.SetFlagRequired(importCmd, "file", "cmd/store/import", false); err != nil {
+		fmt.Print(err)
 		os.Exit(1)
 	}
 }
diff --git a/internal/flags/flags.go b/internal/flags/flags.go
@@ -0,0 +1,44 @@
+// Package flags provides utility functions for working with cobra command flags.
+// It simplifies the process of marking flags as required and handling related errors.
+package flags
+
+import (
+	"errors"
+	"fmt"
+
+	"github.com/spf13/cobra"
+)
+
+const flagRequiredErrorMsg = "error setting %s flag as required - %s: %v"
+
+// SetFlagRequired marks a single flag as required for a cobra command.
+// If isPersistent is true, it marks the persistent flag as required.
+// Returns an error if the flag cannot be marked as required.
+func SetFlagRequired(cmd *cobra.Command, flag string, location string, isPersistent bool) error {
+	if isPersistent {
+		if err := cmd.MarkPersistentFlagRequired(flag); err != nil {
+			return fmt.Errorf(flagRequiredErrorMsg, flag, location, err)
+		}
+	} else {
+		if err := cmd.MarkFlagRequired(flag); err != nil {
+			return fmt.Errorf(flagRequiredErrorMsg, flag, location, err)
+		}
+	}
+
+	return nil
+}
+
+// SetFlagsRequired marks multiple flags as required for a cobra command.
+// If isPersistent is true, it marks the persistent flags as required.
+// Returns a joined error if any flag cannot be marked as required.
+func SetFlagsRequired(cmd *cobra.Command, flags []string, location string, isPersistent bool) error {
+	flagErrors := make([]error, len(flags))
+
+	for i, flag := range flags {
+		if err := SetFlagRequired(cmd, flag, location, isPersistent); err != nil {
+			flagErrors[i] = err
+		}
+	}
+
+	return errors.Join(flagErrors...)
+}
diff --git a/internal/flags/flags_test.go b/internal/flags/flags_test.go
@@ -0,0 +1,149 @@
+package flags
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSetFlagRequired_NonPersistent_Success(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+	cmd.Flags().String("foo", "", "foo flag")
+
+	err := SetFlagRequired(cmd, "foo", "TestLocation", false)
+
+	assert.NoError(t, err)
+}
+
+func TestSetFlagRequired_Persistent_Success(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+	cmd.PersistentFlags().String("bar", "", "bar flag")
+
+	err := SetFlagRequired(cmd, "bar", "TestLocation", true)
+
+	assert.NoError(t, err)
+}
+
+func TestSetFlagRequired_NonPersistent_FlagNotFound(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+
+	err := SetFlagRequired(cmd, "missing", "TestLocation", false)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "error setting missing flag as required - TestLocation:")
+}
+
+func TestSetFlagRequired_Persistent_FlagNotFound(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+
+	err := SetFlagRequired(cmd, "missing", "TestLocation", true)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "error setting missing flag as required - TestLocation:")
+}
+
+func TestSetFlagRequired_ErrorMessageFormat(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+	flagName := "nonexistent"
+	location := "SomeFunction"
+
+	err := SetFlagRequired(cmd, flagName, location, false)
+
+	require.Error(t, err)
+	expectedPrefix := fmt.Sprintf("error setting %s flag as required - %s:", flagName, location)
+	assert.Contains(t, err.Error(), expectedPrefix)
+}
+
+func TestSetFlagsRequired_AllSuccess(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+	cmd.Flags().String("foo", "", "foo flag")
+	cmd.Flags().String("bar", "", "bar flag")
+	cmd.Flags().String("baz", "", "baz flag")
+
+	flags := []string{"foo", "bar", "baz"}
+	err := SetFlagsRequired(cmd, flags, "TestLocation", false)
+
+	assert.NoError(t, err)
+}
+
+func TestSetFlagsRequired_PersistentAllSuccess(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+	cmd.PersistentFlags().String("foo", "", "foo flag")
+	cmd.PersistentFlags().String("bar", "", "bar flag")
+
+	flags := []string{"foo", "bar"}
+	err := SetFlagsRequired(cmd, flags, "TestLocation", true)
+
+	assert.NoError(t, err)
+}
+
+func TestSetFlagsRequired_SomeSuccess_SomeFail(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+	cmd.Flags().String("foo", "", "foo flag")
+	// "missing" flag is not defined
+
+	flags := []string{"foo", "missing"}
+	err := SetFlagsRequired(cmd, flags, "TestLocation", false)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "error setting missing flag as required - TestLocation:")
+	assert.NotContains(t, err.Error(), "error setting foo flag as required")
+}
+
+func TestSetFlagsRequired_AllFail(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+
+	flags := []string{"missing1", "missing2"}
+	err := SetFlagsRequired(cmd, flags, "TestLocation", false)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "error setting missing1 flag as required - TestLocation:")
+	assert.Contains(t, err.Error(), "error setting missing2 flag as required - TestLocation:")
+}
+
+func TestSetFlagsRequired_EmptySlice(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+
+	err := SetFlagsRequired(cmd, []string{}, "TestLocation", false)
+
+	assert.NoError(t, err)
+}
+
+func TestSetFlagsRequired_NilSlice(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+
+	err := SetFlagsRequired(cmd, nil, "TestLocation", false)
+
+	assert.NoError(t, err)
+}
+
+func TestSetFlagsRequired_ErrorJoining(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+
+	flags := []string{"missing1", "missing2", "missing3"}
+	err := SetFlagsRequired(cmd, flags, "TestLocation", false)
+
+	require.Error(t, err)
+
+	for _, flag := range flags {
+		expectedError := fmt.Sprintf("error setting %s flag as required - TestLocation:", flag)
+		assert.Contains(t, err.Error(), expectedError)
+	}
+}
+
+func TestSetFlagsRequired_MixedPersistentAndNonPersistent(t *testing.T) {
+	cmd := &cobra.Command{Use: "test"}
+	cmd.Flags().String("regular", "", "regular flag")
+	cmd.PersistentFlags().String("persistent", "", "persistent flag")
+
+	flags := []string{"regular", "persistent"}
+	err := SetFlagsRequired(cmd, flags, "TestLocation", false)
+
+	// The regular flag should succeed, but persistent flag should fail
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "error setting persistent flag as required - TestLocation:")
+	assert.NotContains(t, err.Error(), "error setting regular flag as required")
+}

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@ import (`
`24`	`24`
`25`	`25`	`"github.com/openfga/cli/internal/authorizationmodel"`
`26`	`26`	`"github.com/openfga/cli/internal/cmdutils"`
	`27`	`+ "github.com/openfga/cli/internal/flags"`
`27`	`28`	`"github.com/openfga/cli/internal/output"`
`28`	`29`	`)`
`29`	`30`
`@@ -77,8 +78,8 @@ func init() {`
`77`	`78`	`getCmd.Flags().StringArray("field", []string{"model"}, "Fields to display, choices are: id, created_at and model") //nolint:lll`
`78`	`79`	getCmd.Flags().Var(&getOutputFormat, "format", `Authorization model output format. Can be "fga" or "json"`)
`79`	`80`
`80`		`- if err := getCmd.MarkFlagRequired("store-id"); err != nil {`
`81`		`- fmt.Printf("error setting flag as required - %v: %v\n", "cmd/models/get", err)`
	`81`	`+ if err := flags.SetFlagRequired(getCmd, "store-id", "cmd/models/get", false); err != nil {`
	`82`	`+ fmt.Print(err)`
`82`	`83`	`os.Exit(1)`
`83`	`84`	`}`
`84`	`85`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,7 @@ import (`
`27`	`27`
`28`	`28`	`"github.com/openfga/cli/internal/authorizationmodel"`
`29`	`29`	`"github.com/openfga/cli/internal/cmdutils"`
	`30`	`+ "github.com/openfga/cli/internal/flags"`
`30`	`31`	`"github.com/openfga/cli/internal/output"`
`31`	`32`	`)`
`32`	`33`
`@@ -112,8 +113,8 @@ func init() {`
`112`	`113`	`listCmd.Flags().String("store-id", "", "Store ID")`
`113`	`114`	`listCmd.Flags().StringArray("field", []string{"id", "created_at"}, "Fields to display, choices are: id, created_at and model") //nolint:lll`
`114`	`115`
`115`		`- if err := listCmd.MarkFlagRequired("store-id"); err != nil {`
`116`		`- fmt.Printf("error setting flag as required - %v: %v\n", "cmd/models/list", err)`
	`116`	`+ if err := flags.SetFlagRequired(listCmd, "store-id", "cmd/models/list", false); err != nil {`
	`117`	`+ fmt.Print(err)`
`117`	`118`	`os.Exit(1)`
`118`	`119`	`}`
`119`	`120`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,6 +25,7 @@ import (`
`25`	`25`
`26`	`26`	`"github.com/openfga/cli/internal/cmdutils"`
`27`	`27`	`"github.com/openfga/cli/internal/confirmation"`
	`28`	`+ "github.com/openfga/cli/internal/flags"`
`28`	`29`	`"github.com/openfga/cli/internal/output"`
`29`	`30`	`)`
`30`	`31`
`@@ -72,8 +73,7 @@ func init() {`
`72`	`73`	`deleteCmd.Flags().String("store-id", "", "Store ID")`
`73`	`74`	`deleteCmd.Flags().Bool("force", false, "Force delete without confirmation")`
`74`	`75`
`75`		`- err := deleteCmd.MarkFlagRequired("store-id")`
`76`		`- if err != nil {`
	`76`	`+ if err := flags.SetFlagRequired(deleteCmd, "store-id", "cmd/store/delete", false); err != nil {`
`77`	`77`	`fmt.Print(err)`
`78`	`78`	`os.Exit(1)`
`79`	`79`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,7 @@ import (`
`26`	`26`
`27`	`27`	`"github.com/openfga/cli/internal/cmdutils"`
`28`	`28`	`"github.com/openfga/cli/internal/fga"`
	`29`	`+ "github.com/openfga/cli/internal/flags"`
`29`	`30`	`"github.com/openfga/cli/internal/output"`
`30`	`31`	`)`
`31`	`32`
`@@ -65,8 +66,7 @@ var getCmd = &cobra.Command{`
`65`	`66`	`func init() {`
`66`	`67`	`getCmd.Flags().String("store-id", "", "Store ID")`
`67`	`68`
`68`		`- err := getCmd.MarkFlagRequired("store-id")`
`69`		`- if err != nil {`
	`69`	`+ if err := flags.SetFlagRequired(getCmd, "store-id", "cmd/store/get", false); err != nil {`
`70`	`70`	`fmt.Print(err)`
`71`	`71`	`os.Exit(1)`
`72`	`72`	`}`