From 74b8bb812ae26df941fb5bd7121598d38f156aec Mon Sep 17 00:00:00 2001 From: Kendall Garner <17521368+kgarner7@users.noreply.github.com> Date: Mon, 1 Dec 2025 19:45:55 -0800 Subject: [PATCH] address initial comments --- cmd/pls.go | 2 +- cmd/user.go | 63 ++++++++++++++++++++++++++++++++++++++++------------ cmd/utils.go | 7 +++--- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/cmd/pls.go b/cmd/pls.go index 9fe1f96c5..5d6a05b60 100644 --- a/cmd/pls.go +++ b/cmd/pls.go @@ -103,7 +103,7 @@ func runList() { options := model.QueryOptions{Sort: "owner_name"} if userID != "" { - user, err := getUser(userID, ds, ctx) + user, err := getUser(ctx, userID, ds) if err != nil { log.Fatal(ctx, "Error retrieving user", "username or id", userID) } diff --git a/cmd/user.go b/cmd/user.go index 6650721ee..10ee0d543 100644 --- a/cmd/user.go +++ b/cmd/user.go @@ -36,18 +36,15 @@ func init() { userCreateCommand.Flags().StringVarP(&userID, "username", "u", "", "username") - userCreateCommand.Flags().BoolVar(&setPassword, "set-password", false, "If set, the user's new password will be prompted on the CLI") userCreateCommand.Flags().StringVarP(&password, "password", "p", "", "Set the user's password. Note that this will be captured in terminal history") - userCreateCommand.MarkFlagsMutuallyExclusive("password", "set-password") userCreateCommand.Flags().StringVarP(&email, "email", "e", "", "New user email") - userCreateCommand.Flags().IntSliceVar(&libraryIds, "library-ids", []int{}, "Set the user's accessible libraries. If empty, the user can access all libraries. This is incompatible with admin, as admin can always access all libraries") + userCreateCommand.Flags().IntSliceVarP(&libraryIds, "library-ids", "i", []int{}, "Comma-separated list of library IDs. Set the user's accessible libraries. If empty, the user can access all libraries. This is incompatible with admin, as admin can always access all libraries") userCreateCommand.Flags().BoolVarP(&setAdmin, "admin", "a", false, "If set, make the user an admin. This user will have access to every library") userCreateCommand.Flags().StringVar(&name, "name", "", "New user's name (this is separate from username used to log in)") _ = userCreateCommand.MarkFlagRequired("username") - userCreateCommand.MarkFlagsOneRequired("password", "set-password") userRoot.AddCommand(userCreateCommand) @@ -73,7 +70,7 @@ func init() { userEditCommand.Flags().StringVarP(&password, "password", "p", "", "Set the user's password. Note that this will be captured in terminal history") userEditCommand.MarkFlagsMutuallyExclusive("password", "set-password") - userEditCommand.Flags().IntSliceVar(&libraryIds, "library-ids", []int{}, "Set the user's accessible libraries by id") + userEditCommand.Flags().IntSliceVarP(&libraryIds, "library-ids", "i", []int{}, "Comma-separated list of library IDs. Set the user's accessible libraries by id") _ = userEditCommand.MarkFlagRequired("user") userRoot.AddCommand(userEditCommand) @@ -161,6 +158,14 @@ func promptPassword() string { } } +func libraryError(libraries model.Libraries) error { + ids := make([]int, len(libraries)) + for idx, library := range libraries { + ids[idx] = library.ID + } + return fmt.Errorf("not all available libraries found. Requested ids: %v, Found libraries: %v", libraryIds, ids) +} + func runCreateUser() { if password == "" { password = promptPassword() @@ -177,6 +182,10 @@ func runCreateUser() { NewPassword: password, } + if user.Name == "" { + user.Name = userID + } + ds, ctx := getContext() err := ds.WithTx(func(tx model.DataStore) error { @@ -196,7 +205,7 @@ func runCreateUser() { } if len(user.Libraries) != len(libraryIds) { - return fmt.Errorf("not all available libraries found. Requested ids: %v, Found libraries: %d", libraryIds, len(user.Libraries)) + return libraryError(user.Libraries) } } else { user.Libraries, err = tx.Library(ctx).GetAll() @@ -210,7 +219,13 @@ func runCreateUser() { return err } - return nil + updatedIds := make([]int, len(user.Libraries)) + for idx, lib := range user.Libraries { + updatedIds[idx] = lib.ID + } + + err = tx.User(ctx).SetUserLibraries(user.ID, updatedIds) + return err }) if err != nil { @@ -236,7 +251,7 @@ func runDeleteUser() { return errors.New("refusing to delete the last user") } - user, err = getUser(userID, tx, ctx) + user, err = getUser(ctx, userID, tx) if err != nil { return err } @@ -259,7 +274,9 @@ func runUserEdit() { changes := []string{} err = ds.WithTx(func(tx model.DataStore) error { - user, err = getUser(userID, tx, ctx) + var newLibraries model.Libraries + + user, err = getUser(ctx, userID, tx) if err != nil { return err } @@ -272,10 +289,10 @@ func runUserEdit() { } if len(libraries) != len(libraryIds) { - return fmt.Errorf("not all available libraries found. Requested ids: %v, Found libraries: %d", libraryIds, len(libraries)) + return libraryError(libraries) } - user.Libraries = libraries + newLibraries = libraries changes = append(changes, "updated library ids") } @@ -288,6 +305,8 @@ func runUserEdit() { user.IsAdmin = true user.Libraries = libraries changes = append(changes, "set admin") + + newLibraries = libraries } if setRegularUser && user.IsAdmin { @@ -325,15 +344,31 @@ func runUserEdit() { return nil } - err = tx.User(ctx).Put(user) - return err + err := tx.User(ctx).Put(user) + if err != nil { + return err + } + + if len(newLibraries) > 0 { + updatedIds := make([]int, len(newLibraries)) + for idx, lib := range newLibraries { + updatedIds[idx] = lib.ID + } + + err := tx.User(ctx).SetUserLibraries(user.ID, updatedIds) + if err != nil { + return err + } + } + + return nil }) if err != nil { log.Fatal(ctx, "Failed to update user", err) } - log.Info(ctx, "Updated user", "user", user.Name, "changes", strings.Join(changes, ", ")) + log.Info(ctx, "Updated user", "user", user.UserName, "changes", strings.Join(changes, ", ")) } type displayLibrary struct { diff --git a/cmd/utils.go b/cmd/utils.go index 0b4a33913..2ddc8e30f 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -3,6 +3,7 @@ package cmd import ( "context" "errors" + "fmt" "github.com/navidrome/navidrome/core/auth" "github.com/navidrome/navidrome/db" @@ -16,17 +17,17 @@ func getContext() (model.DataStore, context.Context) { return ds, auth.WithAdminUser(context.Background(), ds) } -func getUser(id string, ds model.DataStore, ctx context.Context) (*model.User, error) { +func getUser(ctx context.Context, id string, ds model.DataStore) (*model.User, error) { user, err := ds.User(ctx).FindByUsername(id) if err != nil && !errors.Is(err, model.ErrNotFound) { - return nil, err + return nil, fmt.Errorf("finding user by name: %w", err) } if errors.Is(err, model.ErrNotFound) { user, err = ds.User(ctx).Get(id) if err != nil { - return nil, err + return nil, fmt.Errorf("finding user by id: %w", err) } }