feedback 2

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)
 		}

cmd/user.go

@@ -22,7 +22,6 @@ var (
 	email      string
 	libraryIds []int
 	name       string
-	password   string
 
 	removeEmail    bool
 	removeName     bool
@@ -36,18 +35,13 @@ 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)
 
@@ -70,10 +64,8 @@ func init() {
 	userEditCommand.MarkFlagsMutuallyExclusive("name", "remove-name")
 
 	userEditCommand.Flags().BoolVar(&setPassword, "set-password", false, "If set, the user's new password will be prompted on the CLI")
-	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,12 +153,18 @@ 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() {
+	password := promptPassword()
 	if password == "" {
-		password = promptPassword()
+		log.Fatal("Empty password provided, user creation cancelled")
-		if password == "" {
-			log.Fatal("Empty password provided, user creation cancelled")
-		}
 	}
 
 	user := model.User{
@@ -177,6 +175,10 @@ func runCreateUser() {
 		NewPassword: password,
 	}
 
+	if user.Name == "" {
+		user.Name = userID
+	}
+
 	ds, ctx := getContext()
 
 	err := ds.WithTx(func(tx model.DataStore) error {
@@ -196,7 +198,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 +212,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 +244,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 +267,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 +282,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 +298,8 @@ func runUserEdit() {
 			user.IsAdmin = true
 			user.Libraries = libraries
 			changes = append(changes, "set admin")
+
+			newLibraries = libraries
 		}
 
 		if setRegularUser && user.IsAdmin {
@@ -296,12 +308,12 @@ func runUserEdit() {
 		}
 
 		if setPassword {
-			password = promptPassword()
+			password := promptPassword()
-		}
 
-		if password != "" {
+			if password != "" {
-			user.NewPassword = password
+				user.NewPassword = password
-			changes = append(changes, "updated password")
+				changes = append(changes, "updated password")
+			}
 		}
 
 		if email != "" && email != user.Email {
@@ -321,19 +333,38 @@ func runUserEdit() {
 		}
 
 		if len(changes) == 0 {
-			log.Info(ctx, "No changes for user", "user", user.UserName)
 			return nil
 		}
 
-		err = tx.User(ctx).Put(user)
+		err := tx.User(ctx).Put(user)
-		return err
+		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, ", "))
+	if len(changes) == 0 {
+		log.Info(ctx, "No changes for user", "user", user.UserName)
+	} else {
+		log.Info(ctx, "Updated user", "user", user.UserName, "changes", strings.Join(changes, ", "))
+	}
 }
 
 type displayLibrary struct {
@@ -413,7 +444,7 @@ func runUserList() {
 				user.UpdatedAt.Format(time.RFC3339Nano),
 				lastAccess,
 				lastLogin,
-				fmt.Sprintf("'%s'", strings.Join(paths, ",")),
+				fmt.Sprintf("'%s'", strings.Join(paths, "|")),
 			})
 		}
 		w.Flush()

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)
 		}
 	}