bug: Redundant API calls for user deletion #697
Description
Refactor: Optimize User Deletion and Improve Error Handling
Description
The current implementation of the user delete command is inefficient when deleting multiple users. It performs redundant API calls and lacks standardized error reporting via RunE.
Current Issues
- Redundant API Calls: For every username passed in the arguments, the command calls
GetUsersIdByName. This function fetches the entire list of users every single time it is called.
for _, arg := range args {
// Retrieve user ID by name.
userID, err := api.GetUsersIdByName(arg)
if err != nil {
log.Errorf("failed to get user id for '%s': %v", arg, err)
continue
}
wg.Add(1)
go func(userID int64) {
defer wg.Done()
if err := api.DeleteUser(userID); err != nil {
errChan <- err
}
}(userID)
}
// Inside GetUsersIdByName
u, err := ListUsers(opts) // Fetches ALL users
for _, user := range u.Payload {
if user.Username == userName {
return user.UserID, nil
}
}
If a user deletes n accounts, the CLI fetches the full user list n times, making the subsequent concurrent deletion logic pointless due to the slow user id search phase.
- Inconsistent Output:
The command does not useRunEor a standardized view to report success and failure messages, which is inconsistent with newer delete commands in the CLI.
Proposed Changes
- Batch Lookup: Fetch the full list of users only once at the start of the command.
- Standardize UI: Use
RunEand clear success/error messaging (similar to the logic used in newer delete command implementations).