diff --git a/cmd/mcp/tools_users.go b/cmd/mcp/tools_users.go index 6dd7454..9f15a21 100644 --- a/cmd/mcp/tools_users.go +++ b/cmd/mcp/tools_users.go @@ -96,15 +96,22 @@ func UsersUpdateHandler() server.ToolHandlerFunc { return nil, err } body := api.UserUpdatePayload{} + changed := false if v := req.GetString("email", ""); v != "" { body.SetEmail(v) + changed = true } if v := req.GetString("username", ""); v != "" { body.SetUsername(v) + changed = true } if v := req.GetFloat("permissions", -1); v >= 0 { f := float32(v) body.SetPermissions(f) + changed = true + } + if !changed { + return mcp.NewToolResultError("at least one field must be provided"), nil } client := newAPIClientWithKey(apiKeyFromContext(callCtx)) res, _, err := client.UsersAPI.UserUserIdPut(callCtx, float32(userId)).UserUpdatePayload(body).Execute() diff --git a/cmd/request/update.go b/cmd/request/update.go index 689cf9e..709abc5 100644 --- a/cmd/request/update.go +++ b/cmd/request/update.go @@ -21,6 +21,7 @@ var updateCmd = &cobra.Command{ mediaType, _ := cmd.Flags().GetString("media-type") body := *api.NewRequestRequestIdPutRequest(mediaType) + changed := false if cmd.Flags().Changed("seasons") { v, _ := cmd.Flags().GetString("seasons") parts := strings.Split(v, ",") @@ -33,30 +34,40 @@ var updateCmd = &cobra.Command{ nums = append(nums, float32(n)) } body.SetSeasons(nums) + changed = true } if cmd.Flags().Changed("is4k") { v, _ := cmd.Flags().GetBool("is4k") body.SetIs4k(v) + changed = true } if cmd.Flags().Changed("server-id") { v, _ := cmd.Flags().GetFloat32("server-id") body.SetServerId(v) + changed = true } if cmd.Flags().Changed("profile-id") { v, _ := cmd.Flags().GetFloat32("profile-id") body.SetProfileId(v) + changed = true } if cmd.Flags().Changed("root-folder") { v, _ := cmd.Flags().GetString("root-folder") body.SetRootFolder(v) + changed = true } if cmd.Flags().Changed("language-profile-id") { v, _ := cmd.Flags().GetFloat32("language-profile-id") body.SetLanguageProfileId(v) + changed = true } if cmd.Flags().Changed("user-id") { v, _ := cmd.Flags().GetFloat32("user-id") body.SetUserId(v) + changed = true + } + if !changed { + return fmt.Errorf("at least one field must be provided") } res, r, err := apiClient.RequestAPI.RequestRequestIdPut(ctx, args[0]).RequestRequestIdPutRequest(body).Execute() diff --git a/cmd/users/update.go b/cmd/users/update.go index 67044c7..86923f3 100644 --- a/cmd/users/update.go +++ b/cmd/users/update.go @@ -28,14 +28,21 @@ var updateCmd = &cobra.Command{ permissions, _ := cmd.Flags().GetFloat32("permissions") body := api.UserUpdatePayload{} + changed := false if cmd.Flags().Changed("username") { body.Username = &username + changed = true } if cmd.Flags().Changed("email") { body.Email = &email + changed = true } if cmd.Flags().Changed("permissions") { body.Permissions = &permissions + changed = true + } + if !changed { + return fmt.Errorf("at least one field must be provided") } res, r, err := apiClient.UsersAPI.UserUserIdPut(ctx, float32(userId)).UserUpdatePayload(body).Execute() diff --git a/tests/request_test.go b/tests/request_test.go index 887cd4b..893ad85 100644 --- a/tests/request_test.go +++ b/tests/request_test.go @@ -99,7 +99,7 @@ func TestRequest(t *testing.T) { {"create", []string{"request", "create", "--media-type", "movie", "--media-id", "123"}, `"id": 2`}, {"count", []string{"request", "count"}, `"total": 5`}, {"get", []string{"request", "get", "1"}, `"id": 1`}, - {"update", []string{"request", "update", "1", "--media-type", "movie"}, `"id": 1`}, + {"update", []string{"request", "update", "1", "--media-type", "movie", "--user-id", "5"}, `"id": 1`}, {"delete", []string{"request", "delete", "1"}, `"status": "ok"`}, {"retry", []string{"request", "retry", "1"}, `"id": 1`}, {"approve", []string{"request", "approve", "1"}, `"status": 2`}, diff --git a/tests/update_empty_payload_test.go b/tests/update_empty_payload_test.go new file mode 100644 index 0000000..77d1c2b --- /dev/null +++ b/tests/update_empty_payload_test.go @@ -0,0 +1,85 @@ +package tests + +import ( + "bytes" + "context" + "strings" + "testing" + + "seerr-cli/cmd" + "seerr-cli/cmd/apiutil" + cmdmcp "seerr-cli/cmd/mcp" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/spf13/pflag" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// resetCmdFlags resets the Changed state of all flags on the named subcommand. +// Cobra does not reset Changed between Execute() calls, so this helper prevents +// flag state from leaking between tests that share the global command tree. +func resetCmdFlags(path ...string) { + if c, _, _ := cmd.RootCmd.Find(path); c != nil { + c.Flags().VisitAll(func(f *pflag.Flag) { f.Changed = false }) + } +} + +// TestUsersUpdateEmptyPayload verifies that users update returns an error when +// no optional fields are provided so that the API is never called with an empty body. +func TestUsersUpdateEmptyPayload(t *testing.T) { + resetCmdFlags("users", "update") + + // Point at a dummy server; it must not receive any requests. + apiutil.OverrideServerURL = "http://127.0.0.1:0/api/v1" + viper.Set("seerr.server", "http://127.0.0.1:0") + defer func() { apiutil.OverrideServerURL = "" }() + + buf := new(bytes.Buffer) + cmd.RootCmd.SetOut(buf) + cmd.RootCmd.SetArgs([]string{"users", "update", "1"}) + + err := cmd.RootCmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "at least one field must be provided") +} + +// TestRequestUpdateEmptyPayload verifies that request update returns an error +// when only the required --media-type flag is provided and no optional fields +// are set, preventing a no-op API call. +func TestRequestUpdateEmptyPayload(t *testing.T) { + resetCmdFlags("request", "update") + + apiutil.OverrideServerURL = "http://127.0.0.1:0/api/v1" + viper.Set("seerr.server", "http://127.0.0.1:0") + defer func() { apiutil.OverrideServerURL = "" }() + + buf := new(bytes.Buffer) + cmd.RootCmd.SetOut(buf) + cmd.RootCmd.SetArgs([]string{"request", "update", "42", "--media-type", "movie"}) + + err := cmd.RootCmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "at least one field must be provided") +} + +// TestMCPUsersUpdateEmptyPayload verifies that the MCP users_update tool +// returns an error result when only userId is provided and no update fields +// are set. +func TestMCPUsersUpdateEmptyPayload(t *testing.T) { + handler := cmdmcp.UsersUpdateHandler() + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]any{ + "userId": float64(1), + } + + result, err := handler(context.Background(), req) + require.NoError(t, err) + require.NotNil(t, result) + assert.True(t, result.IsError, "expected IsError=true for empty payload") + require.NotEmpty(t, result.Content) + textContent, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.True(t, strings.Contains(textContent.Text, "at least one field must be provided")) +}