Artisan command for updating avatars for existing users

[MarcHagen]

• Related: Artisan command for "refetching" user avatars for existing users #3232
• Solves: Artisan command for "refetching" user avatars for existing users. #1893

---

Adding a console command to:

• Update one user's avatar, searching the user by id or email (--email= or --id=)
• Updating all users that do not have an avatar (--users-without-avatars)
  • Default behavior is dry-run
• Updating all users' avatars (--all)
  • Default behavior is dry-run

Adding check for avatar fetching if the response code is OK.

Having the #3232 (comment) in mind.

Please let me know if any changes or refactoring is needed!

---

Console outputs:

# php artisan bookstack:refresh-avatar
Either a --id=<number> or --email=<email> option must be provided.
Run with `--help` to more options

# php artisan bookstack:refresh-avatar --id=123
A user where id=123 could not be found.

# php artisan bookstack:refresh-avatar --id=1
This will refresh the avatar for user:
- ID: 1
- Name: Admin
- Email: admin@admin.com

 Are you sure you want to proceed? (yes/no) [no]:
 > yes

User avatar has been updated.

# php artisan bookstack:refresh-avatar --users-without-avatars
2 user(s) found without avatars.
ID 1 - Not updated
ID 2 - Not updated

Dry run, no avatars have been updated
Run with -f or --force to perform the update

# php artisan bookstack:refresh-avatar --users-without-avatars -f
2 user(s) found without avatars.

 Are you sure you want to refresh avatars of users that do not have one? (yes/no) [no]:
 > yes

ID 1 - Updated
ID 2 - Updated

# php artisan bookstack:refresh-avatar --all
2 user(s) found.
ID 1 - Not updated
ID 2 - Not updated

Dry run, no avatars have been updated
Run with -f or --force to perform the update

# php artisan bookstack:refresh-avatar --all -f
2 user(s) found.

 Are you sure you want to refresh avatars for ALL USERS? (yes/no) [no]:
 > yes

ID 1 - Updated
ID 2 - Updated

[ssddanbrown]

Thanks @MarcHagen, this looks great!
I appreciate the extensive suite of tests provided.

I've assigned to the next release.
I'll look to fully dig in to review and merge in the next few days.

[ssddanbrown]

Thanks again @MarcHagen.
This was all working great from my review.

I did follow this up with 4b4d8ba primarily to simplify the logic of this command, so it follows pretty the same path for all three options (I was nervous about the amount of logic for a command that's had very little demand), and to extract & share the user handling across commands, otherwise it's mostly simplification and formatting/style preference changes.

[MarcHagen]

Awesome stuff, follow up commit looks great. I'm not super familiar with Laravel.

The User::query()->whereDoesntHave('avatar')->get()->all() is something I didn't know about.
Learning every day.

$linePrefix = "[ID: {$user->id}] $user->email -";
This is similar to what I had but ultimately removed because I found it maybe a bit too much info.
But yes this is a better way to log it.

The HandlesSingleUser trait is indeed a good choice to merge similar code.

I am curious why you are using the $this->artisan("bookstack:refresh-avatar --id={$user->id} -f") in this way instead of the $this->artisan(RefreshAvatarCommand::class, ['--id' => $user->id]) way.
Is there a better benefit to it?
For me, letting Laravel resolve the command for you seems better if you ever rename the class

Thank you for the awesome experience you gave me on this PR.

[ssddanbrown]

This is similar to what I had but ultimately removed because I found it maybe a bit too much info.

It can be, but I thought it's important to show users some quick-identifiable info about the user before they take action, especially since I merged the single-user flow into the bulk flow, since you previously had more info there (which I think is important for running for single user).

For me, letting Laravel resolve the command for you seems better if you ever rename the class

I have a habit of hard-coding stuff in tests quite a lot. Including text strings, URLs, expectations and commands.
For commands like this, I like having the test reflect what the user would enter, and the args provided with the string format to avoid any confusion to arg types/conversion. It also makes the API (command name in this case) covered by the testing. If the command name is accidentally changed, the tests would flag it.

Ultimately, it's a preference. There'd always be a balance of purposeful test fragility and maintenance, and this is the balance I've landed on.