From 3a9caea84609c9cf584ba9c15eed5d2a0db5a6d3 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 8 Mar 2021 22:34:22 +0000 Subject: [PATCH] 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'); + }); + } +}