From 3a9caea84609c9cf584ba9c15eed5d2a0db5a6d3 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 8 Mar 2021 22:34:22 +0000 Subject: [PATCH 1/5] Started work on user slugs Related to #2525 --- app/Auth/User.php | 34 +++++-------- app/Entities/Models/Entity.php | 3 +- app/Entities/Tools/SlugGenerator.php | 23 +++++---- app/Interfaces/Sluggable.php | 18 +++++++ database/factories/ModelFactory.php | 4 +- .../2021_03_08_215138_add_user_slug.php | 50 +++++++++++++++++++ 6 files changed, 98 insertions(+), 34 deletions(-) create mode 100644 app/Interfaces/Sluggable.php create mode 100644 database/migrations/2021_03_08_215138_add_user_slug.php diff --git a/app/Auth/User.php b/app/Auth/User.php index 9d2210101..07232e1cc 100644 --- a/app/Auth/User.php +++ b/app/Auth/User.php @@ -2,6 +2,7 @@ use BookStack\Api\ApiToken; use BookStack\Interfaces\Loggable; +use BookStack\Interfaces\Sluggable; use BookStack\Model; use BookStack\Notifications\ResetPassword; use BookStack\Uploads\Image; @@ -22,6 +23,7 @@ use Illuminate\Support\Collection; * Class User * @property string $id * @property string $name + * @property string $slug * @property string $email * @property string $password * @property Carbon $created_at @@ -32,7 +34,7 @@ use Illuminate\Support\Collection; * @property string $system_name * @property Collection $roles */ -class User extends Model implements AuthenticatableContract, CanResetPasswordContract, Loggable +class User extends Model implements AuthenticatableContract, CanResetPasswordContract, Loggable, Sluggable { use Authenticatable, CanResetPassword, Notifiable; @@ -73,23 +75,21 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Returns the default public user. - * @return User */ - public static function getDefault() + public static function getDefault(): User { if (!is_null(static::$defaultUser)) { return static::$defaultUser; } - static::$defaultUser = static::where('system_name', '=', 'public')->first(); + static::$defaultUser = static::query()->where('system_name', '=', 'public')->first(); return static::$defaultUser; } /** * Check if the user is the default public user. - * @return bool */ - public function isDefault() + public function isDefault(): bool { return $this->system_name === 'public'; } @@ -116,12 +116,10 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Check if the user has a role. - * @param $role - * @return mixed */ - public function hasSystemRole($role) + public function hasSystemRole(string $roleSystemName): bool { - return $this->roles->pluck('system_name')->contains($role); + return $this->roles->pluck('system_name')->contains($roleSystemName); } /** @@ -185,9 +183,8 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Get the social account associated with this user. - * @return HasMany */ - public function socialAccounts() + public function socialAccounts(): HasMany { return $this->hasMany(SocialAccount::class); } @@ -208,11 +205,9 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon } /** - * Returns the user's avatar, - * @param int $size - * @return string + * Returns a URL to the user's avatar */ - public function getAvatar($size = 50) + public function getAvatar(int $size = 50): string { $default = url('/user_avatar.png'); $imageId = $this->image_id; @@ -230,9 +225,8 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Get the avatar for the user. - * @return BelongsTo */ - public function avatar() + public function avatar(): BelongsTo { return $this->belongsTo(Image::class, 'image_id'); } @@ -277,10 +271,8 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Get a shortened version of the user's name. - * @param int $chars - * @return string */ - public function getShortName($chars = 8) + public function getShortName(int $chars = 8): string { if (mb_strlen($this->name) <= $chars) { return $this->name; diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index c6b2468b0..f5f43fe8c 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -9,6 +9,7 @@ use BookStack\Auth\Permissions\JointPermission; use BookStack\Entities\Tools\SearchIndex; use BookStack\Entities\Tools\SlugGenerator; use BookStack\Facades\Permissions; +use BookStack\Interfaces\Sluggable; use BookStack\Model; use BookStack\Traits\HasCreatorAndUpdater; use BookStack\Traits\HasOwner; @@ -37,7 +38,7 @@ use Illuminate\Database\Eloquent\SoftDeletes; * @method static Builder withLastView() * @method static Builder withViewCount() */ -abstract class Entity extends Model +abstract class Entity extends Model implements Sluggable { use SoftDeletes; use HasCreatorAndUpdater; diff --git a/app/Entities/Tools/SlugGenerator.php b/app/Entities/Tools/SlugGenerator.php index 7075bc72c..4501279f2 100644 --- a/app/Entities/Tools/SlugGenerator.php +++ b/app/Entities/Tools/SlugGenerator.php @@ -1,6 +1,7 @@ formatNameAsSlug($entity->name); - while ($this->slugInUse($slug, $entity)) { - $slug .= '-' . substr(md5(rand(1, 500)), 0, 3); + $slug = $this->formatNameAsSlug($model->name); + while ($this->slugInUse($slug, $model)) { + $slug .= '-' . Str::random(3); } return $slug; } @@ -35,16 +36,16 @@ class SlugGenerator * Check if a slug is already in-use for this * type of model within the same parent. */ - protected function slugInUse(string $slug, Entity $entity): bool + protected function slugInUse(string $slug, Sluggable $model): bool { - $query = $entity->newQuery()->where('slug', '=', $slug); + $query = $model->newQuery()->where('slug', '=', $slug); - if ($entity instanceof BookChild) { - $query->where('book_id', '=', $entity->book_id); + if ($model instanceof BookChild) { + $query->where('book_id', '=', $model->book_id); } - if ($entity->id) { - $query->where('id', '!=', $entity->id); + if ($model->id) { + $query->where('id', '!=', $model->id); } return $query->count() > 0; diff --git a/app/Interfaces/Sluggable.php b/app/Interfaces/Sluggable.php new file mode 100644 index 000000000..84f0e5bcd --- /dev/null +++ b/app/Interfaces/Sluggable.php @@ -0,0 +1,18 @@ +define(\BookStack\Auth\User::class, function ($faker) { + $name = $faker->name; return [ - 'name' => $faker->name, + 'name' => $name, 'email' => $faker->email, + 'slug' => \Illuminate\Support\Str::slug($name . '-' . \Illuminate\Support\Str::random(5)), 'password' => Str::random(10), 'remember_token' => Str::random(10), 'email_confirmed' => 1 diff --git a/database/migrations/2021_03_08_215138_add_user_slug.php b/database/migrations/2021_03_08_215138_add_user_slug.php new file mode 100644 index 000000000..906e06b95 --- /dev/null +++ b/database/migrations/2021_03_08_215138_add_user_slug.php @@ -0,0 +1,50 @@ +string('slug', 250); + }); + + $slugMap = []; + DB::table('users')->cursor()->each(function ($user) use (&$slugMap) { + $userSlug = Str::slug($user->name); + while (isset($slugMap[$userSlug])) { + $userSlug = Str::slug($user->name . Str::random(4)); + } + $slugMap[$userSlug] = true; + + DB::table('users') + ->where('id', $user->id) + ->update(['slug' => $userSlug]); + }); + + Schema::table('users', function (Blueprint $table) { + $table->unique('slug'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('users', function (Blueprint $table) { + $table->dropColumn('slug'); + }); + } +} From 19d79b6a0f034ab4f7dbd75d60030d276a0c21de Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 9 Mar 2021 23:06:12 +0000 Subject: [PATCH 2/5] Started rolling out user slugs to model and core controllers --- app/Auth/User.php | 12 ++++++- app/Auth/UserRepo.php | 8 +++++ app/Entities/Models/Entity.php | 4 +-- app/Http/Controllers/UserController.php | 36 +++++++------------ .../Controllers/UserProfileController.php | 25 +++++++++++++ app/Interfaces/Sluggable.php | 5 +++ resources/views/common/header.blade.php | 4 +-- routes/web.php | 2 +- 8 files changed, 67 insertions(+), 29 deletions(-) create mode 100644 app/Http/Controllers/UserProfileController.php diff --git a/app/Auth/User.php b/app/Auth/User.php index 07232e1cc..9855ab4e7 100644 --- a/app/Auth/User.php +++ b/app/Auth/User.php @@ -1,6 +1,7 @@ id); + return url('/user/' . $this->slug); } /** @@ -303,4 +304,13 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon { return "({$this->id}) {$this->name}"; } + + /** + * @inheritDoc + */ + public function refreshSlug(): string + { + $this->slug = app(SlugGenerator::class)->generate($this); + return $this->slug; + } } diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php index 29a0ebc14..bcaf9cd09 100644 --- a/app/Auth/UserRepo.php +++ b/app/Auth/UserRepo.php @@ -45,6 +45,14 @@ class UserRepo return User::query()->findOrFail($id); } + /** + * Get a user by their slug. + */ + public function getBySlug(string $slug): User + { + return User::query()->where('slug', '=', $slug)->firstOrFail(); + } + /** * Get all the users with their permissions. */ diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index f5f43fe8c..d4b477304 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -290,11 +290,11 @@ abstract class Entity extends Model implements Sluggable } /** - * Generate and set a new URL slug for this model. + * @inheritdoc */ public function refreshSlug(): string { - $this->slug = (new SlugGenerator)->generate($this); + $this->slug = app(SlugGenerator::class)->generate($this); return $this->slug; } } diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 92e1cd8b7..d797552f1 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -5,10 +5,13 @@ use BookStack\Auth\Access\SocialAuthService; use BookStack\Auth\Access\UserInviteService; use BookStack\Auth\User; use BookStack\Auth\UserRepo; +use BookStack\Exceptions\ImageUploadException; use BookStack\Exceptions\UserUpdateException; use BookStack\Uploads\ImageRepo; +use Exception; use Illuminate\Http\Request; use Illuminate\Support\Str; +use Illuminate\Validation\ValidationException; class UserController extends Controller { @@ -61,7 +64,7 @@ class UserController extends Controller /** * Store a newly created user in storage. * @throws UserUpdateException - * @throws \Illuminate\Validation\ValidationException + * @throws ValidationException */ public function store(Request $request) { @@ -90,6 +93,7 @@ class UserController extends Controller $user->external_auth_id = $request->get('external_auth_id'); } + $user->refreshSlug(); $user->save(); if ($sendInvite) { @@ -132,8 +136,8 @@ class UserController extends Controller /** * Update the specified user in storage. * @throws UserUpdateException - * @throws \BookStack\Exceptions\ImageUploadException - * @throws \Illuminate\Validation\ValidationException + * @throws ImageUploadException + * @throws ValidationException */ public function update(Request $request, int $id) { @@ -157,6 +161,11 @@ class UserController extends Controller $user->email = $request->get('email'); } + // Refresh the slug if the user's name has changed + if ($user->isDirty('name')) { + $user->refreshSlug(); + } + // Role updates if (userCan('users-manage') && $request->filled('roles')) { $roles = $request->get('roles'); @@ -216,7 +225,7 @@ class UserController extends Controller /** * Remove the specified user from storage. - * @throws \Exception + * @throws Exception */ public function destroy(Request $request, int $id) { @@ -243,25 +252,6 @@ class UserController extends Controller return redirect('/settings/users'); } - /** - * Show the user profile page - */ - public function showProfilePage($id) - { - $user = $this->userRepo->getById($id); - - $userActivity = $this->userRepo->getActivity($user); - $recentlyCreated = $this->userRepo->getRecentlyCreated($user, 5); - $assetCounts = $this->userRepo->getAssetCounts($user); - - return view('users.profile', [ - 'user' => $user, - 'activity' => $userActivity, - 'recentlyCreated' => $recentlyCreated, - 'assetCounts' => $assetCounts - ]); - } - /** * Update the user's preferred book-list display setting. */ diff --git a/app/Http/Controllers/UserProfileController.php b/app/Http/Controllers/UserProfileController.php new file mode 100644 index 000000000..95e68cb07 --- /dev/null +++ b/app/Http/Controllers/UserProfileController.php @@ -0,0 +1,25 @@ +getBySlug($slug); + + $userActivity = $repo->getActivity($user); + $recentlyCreated = $repo->getRecentlyCreated($user, 5); + $assetCounts = $repo->getAssetCounts($user); + + return view('users.profile', [ + 'user' => $user, + 'activity' => $userActivity, + 'recentlyCreated' => $recentlyCreated, + 'assetCounts' => $assetCounts + ]); + } +} diff --git a/app/Interfaces/Sluggable.php b/app/Interfaces/Sluggable.php index 84f0e5bcd..6aa94e69c 100644 --- a/app/Interfaces/Sluggable.php +++ b/app/Interfaces/Sluggable.php @@ -15,4 +15,9 @@ use Illuminate\Database\Eloquent\Builder; interface Sluggable { + /** + * Regenerate the slug for this model. + */ + public function refreshSlug(): string; + } \ No newline at end of file diff --git a/resources/views/common/header.blade.php b/resources/views/common/header.blade.php index 43ac273ef..52f6b8cbb 100644 --- a/resources/views/common/header.blade.php +++ b/resources/views/common/header.blade.php @@ -58,10 +58,10 @@