From b46ba956d0f9a2d8681bc088f07a30508fda2efe Mon Sep 17 00:00:00 2001 From: Peter Deltchev Date: Sun, 5 Jun 2016 20:05:51 -0700 Subject: [PATCH] Fixes #51: Slugs are now unique and user-changeable. --- app/Commands/SaveAccountSettingsCommand.php | 23 ++-- .../Controllers/Api/Web/AccountController.php | 11 +- app/Http/routes.php | 1 + app/Library/PfmValidator.php | 114 ++++++++++++++++++ app/Models/User.php | 34 +++++- ...2016_06_05_193032_enforce_unique_slugs.php | 94 +++++++++++++++ public/templates/account/settings.html | 14 ++- public/templates/artists/_show_layout.html | 10 +- .../app/controllers/account-settings.coffee | 26 +++- .../scripts/app/controllers/artist.coffee | 12 +- .../assets/scripts/app/services/auth.coffee | 13 ++ resources/lang/en/validation.php | 1 + 12 files changed, 327 insertions(+), 26 deletions(-) create mode 100644 database/migrations/2016_06_05_193032_enforce_unique_slugs.php diff --git a/app/Commands/SaveAccountSettingsCommand.php b/app/Commands/SaveAccountSettingsCommand.php index 6531e7dc..a4140f48 100644 --- a/app/Commands/SaveAccountSettingsCommand.php +++ b/app/Commands/SaveAccountSettingsCommand.php @@ -37,6 +37,8 @@ class SaveAccountSettingsCommand extends CommandBase { $this->_input = $input; $this->_slug = $slug; + + /** @var User _user */ $this->_user = null; $this->_current = null; } @@ -82,14 +84,17 @@ class SaveAccountSettingsCommand extends CommandBase } $rules = [ - 'display_name' => 'required|min:3|max:26', - 'bio' => 'textarea_length:250' + 'display_name' => 'required|min:3|max:26', + 'bio' => 'textarea_length:250', + 'slug' => [ + 'required', + 'unique:users,slug,'.$this->_user->id, + 'min:3', + 'regex:/^[a-z\d-]+$/', + 'is_not_reserved_slug' + ] ]; - if ($this->_input['sync_names'] == 'true') { - $this->_input['display_name'] = $this->_user->username; - } - if ($this->_input['uses_gravatar'] == 'true') { $rules['gravatar'] = 'email'; } else { @@ -97,7 +102,9 @@ class SaveAccountSettingsCommand extends CommandBase $rules['avatar_id'] = 'exists:images,id'; } - $validator = Validator::make($this->_input, $rules); + $validator = Validator::make($this->_input, $rules, [ + 'slug.regex' => 'Slugs can only contain numbers, lowercase letters, and dashes.' + ]); if ($validator->fails()) { return CommandResponse::fail($validator); @@ -114,7 +121,7 @@ class SaveAccountSettingsCommand extends CommandBase $this->_user->bio = $this->_input['bio']; $this->_user->display_name = $this->_input['display_name']; - $this->_user->sync_names = $this->_input['sync_names'] == 'true'; + $this->_user->slug = $this->_input['slug']; $this->_user->can_see_explicit_content = $this->_input['can_see_explicit_content'] == 'true'; $this->_user->uses_gravatar = $this->_input['uses_gravatar'] == 'true'; diff --git a/app/Http/Controllers/Api/Web/AccountController.php b/app/Http/Controllers/Api/Web/AccountController.php index 70f6fe40..86eedaa7 100644 --- a/app/Http/Controllers/Api/Web/AccountController.php +++ b/app/Http/Controllers/Api/Web/AccountController.php @@ -31,6 +31,15 @@ use Illuminate\Support\Facades\Response; class AccountController extends ApiControllerBase { + public function getUser(User $user) + { + $this->authorize('edit', $user); + + return Response::json([ + 'user' => $user->toArray() + ]); + } + public function getSettings($slug) { $user = null; @@ -58,7 +67,7 @@ class AccountController extends ApiControllerBase 'bio' => $user->bio, 'can_see_explicit_content' => $user->can_see_explicit_content == 1, 'display_name' => $user->display_name, - 'sync_names' => $user->sync_names == 1, + 'slug' => $user->slug, 'username' => $user->username, 'gravatar' => $user->gravatar ? $user->gravatar : $user->email, 'avatar_url' => !$user->uses_gravatar ? $user->getAvatarUrl() : null, diff --git a/app/Http/routes.php b/app/Http/routes.php index 560647f0..9ad49976 100644 --- a/app/Http/routes.php +++ b/app/Http/routes.php @@ -142,6 +142,7 @@ Route::group(['prefix' => 'api/web'], function() { Route::get('/tracks/owned', 'Api\Web\TracksController@getOwned'); Route::get('/tracks/edit/{id}', 'Api\Web\TracksController@getEdit'); + Route::get('/users/{userId}', 'Api\Web\AccountController@getUser');//->where('userId', '\d+'); Route::get('/users/{userId}/albums', 'Api\Web\AlbumsController@getOwned')->where('id', '\d+'); Route::get('/users/{userId}/images', 'Api\Web\ImagesController@getOwned')->where('id', '\d+'); diff --git a/app/Library/PfmValidator.php b/app/Library/PfmValidator.php index 9b4ce6db..508f2d4d 100644 --- a/app/Library/PfmValidator.php +++ b/app/Library/PfmValidator.php @@ -21,6 +21,104 @@ use Illuminate\Support\Str; class PfmValidator extends Illuminate\Validation\Validator { + private static $reservedNames = [ + 'about' => null, + 'account' => null, + 'accounts' => null, + 'admin' => null, + 'administration' => null, + 'administrator' => null, + 'admins' => null, + 'album' => null, + 'albums' => null, + 'animation' => null, + 'animations' => null, + 'api' => null, + 'articles' => null, + 'artist' => null, + 'artists' => null, + 'audio' => null, + 'aurora-gleam' => null, + 'auth' => null, + 'azura' => null, + 'blog' => null, + 'blogs' => null, + 'book' => null, + 'books' => null, + 'buffy' => null, + 'comment' => null, + 'comments' => null, + 'create' => null, + 'dev' => null, + 'developer' => null, + 'developers' => null, + 'edit' => null, + 'editor' => null, + 'error' => null, + 'errors' => null, + 'fair-dice' => null, + 'fairdice' => null, + 'faq' => null, + 'favorite' => null, + 'favourite' => null, + 'favourites' => null, + 'follow' => null, + 'followers' => null, + 'following' => null, + 'gemini-star' => null, + 'genre' => null, + 'genres' => null, + 'home' => null, + 'log-out' => null, + 'login' => null, + 'logout' => null, + 'mail' => null, + 'mlp-forums' => null, + 'mlpforums' => null, + 'mlpforums-advertising-program' => null, + 'movie' => null, + 'movies' => null, + 'music' => null, + 'new' => null, + 'news' => null, + 'notification' => null, + 'notifications' => null, + 'nova-blast' => null, + 'page' => null, + 'pages' => null, + 'pixel-wavelength' => null, + 'pixelwavelength' => null, + 'playlist' => null, + 'playlists' => null, + 'poniverse' => null, + 'pony-fm' => null, + 'ponyfm' => null, + 'ponyverse' => null, + 'ponyville-live' => null, + 'ponyvillelive' => null, + 'profile' => null, + 'profiles' => null, + 'register' => null, + 'sign-in' => null, + 'sign-up' => null, + 'signin' => null, + 'signup' => null, + 'template' => null, + 'templates' => null, + 'track' => null, + 'tracks' => null, + 'tunes' => null, + 'upload' => null, + 'uploader' => null, + 'user' => null, + 'users' => null, + 'viridian-meadows' => null, + 'web' => null, + 'word-play' => null, + 'wordplay' => null, + 'www' => null, + ]; + /** * Determine if a given rule implies that the attribute is required. * @@ -195,4 +293,20 @@ class PfmValidator extends Illuminate\Validation\Validator { return strlen(str_replace("\r\n", "\n", $value)) <= $parameters[0]; } + + /** + * This validation rule is intended to avoid collisions between user profile + * slugs and site functionality. + * + * @param $attribute + * @param $value + * @param $parameters + * @return bool false if the given value is a reserved slug + */ + public function validateIsNotReservedSlug($attribute, $value, $parameters) + { + return !array_key_exists($value, static::$reservedNames) && + // Pony.fm shortlinks are in the form: /{letter}{series of numbers} + !preg_match('/^[a-z]?\d+$/', $value); + } } diff --git a/app/Models/User.php b/app/Models/User.php index 964eefbb..6d20005b 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -34,6 +34,7 @@ use Illuminate\Support\Str; use Poniverse\Ponyfm\Contracts\Commentable; use Poniverse\Ponyfm\Contracts\Searchable; use Poniverse\Ponyfm\Traits\IndexedInElasticsearchTrait; +use Validator; use Venturecraft\Revisionable\RevisionableTrait; /** @@ -89,7 +90,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon 'is_archived' => 'boolean', ]; protected $dates = ['created_at', 'updated_at', 'disabled_at']; - protected $hidden = ['disabled_at']; + protected $hidden = ['disabled_at', 'remember_token']; public function scopeUserDetails($query) { @@ -104,6 +105,35 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon return !$query; } + /** + * Takes the given string, slugifies it, and increments a counter if needed + * to generate a unique slug version of it. + * + * @param string $name + * @return string a unique slug + */ + private static function getUniqueSlugForName(string $name):string { + $baseSlug = Str::slug($name); + $slugBeingTried = $baseSlug; + $counter = 2; + + while(true) { + $existingEntity = static::where('slug', $slugBeingTried)->first(); + $validator = Validator::make(['slug' => $slugBeingTried], ['isNotReservedSlug']); + + if ($existingEntity || $validator->fails()) { + $slugBeingTried = "{$baseSlug}-{$counter}"; + $counter++; + continue; + + } else { + break; + } + } + + return $slugBeingTried; + } + /** * @param string $username * @param string $displayName @@ -123,6 +153,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon $user->username = $username; $user->display_name = $displayName; + $user->slug = static::getUniqueSlugForName($displayName); $user->email = $email; $user->uses_gravatar = true; $user->save(); @@ -204,7 +235,6 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon public function setDisplayNameAttribute($value) { $this->attributes['display_name'] = $value; - $this->attributes['slug'] = Str::slug($value); } public function getAvatarUrl($type = Image::NORMAL) diff --git a/database/migrations/2016_06_05_193032_enforce_unique_slugs.php b/database/migrations/2016_06_05_193032_enforce_unique_slugs.php new file mode 100644 index 00000000..86229b5a --- /dev/null +++ b/database/migrations/2016_06_05_193032_enforce_unique_slugs.php @@ -0,0 +1,94 @@ +select(['slug', DB::raw('COUNT(*)')]) + ->groupBy('slug') + ->havingRaw('COUNT(*) > 1') + ->lists('slug'); + + foreach ($slugs as $slug) { + DB::transaction(function () use ($slug) { + // find users with that slug, ordered by published content + $users = DB::table('users') + ->select([ + 'id', + 'slug', + 'disabled_at', + DB::raw('(SELECT COUNT(*) FROM tracks WHERE tracks.user_id = users.id AND published_at IS NOT NULL AND deleted_at IS NULL) as track_count'), + DB::raw('(SELECT COUNT(*) FROM albums WHERE albums.user_id = users.id AND deleted_at IS NULL AND albums.track_count > 1) as album_count'), + DB::raw('(SELECT COUNT(*) FROM playlists WHERE playlists.user_id = users.id AND deleted_at IS NULL AND playlists.track_count > 1) as playlist_count'), + ]) + ->where('slug', $slug) + ->orderBy('disabled_at', 'ASC') + ->orderBy('track_count', 'DESC') + ->orderBy('playlist_count', 'DESC') + ->orderBy('album_count', 'DESC') + ->get(); + + // ensure a unique slug for each + $isOriginalSlugTaken = false; + $counter = 2; + foreach($users as $user) { + if (false === $isOriginalSlugTaken) { + // This lucky user gets to keep the original slug! + $isOriginalSlugTaken = true; + continue; + } else { + $now = \Carbon\Carbon::now(); + $newSlug = "{$slug}-{$counter}"; + + DB::table('revisions') + ->insert([ + 'revisionable_type' => 'Poniverse\\Ponyfm\\Models\\User', + 'revisionable_id' => $user->id, + 'user_id' => null, + 'key' => 'slug', + 'old_value' => $slug, + 'new_value' => $newSlug, + 'created_at' => $now, + 'updated_at' => $now, + ]); + + DB::table('users') + ->where('id', $user->id) + ->update([ + 'slug' => $newSlug, + 'updated_at' => $now + ]); + + $counter++; + } + } + }); + } + + Schema::table('users', function(Blueprint $table) { + $table->unique('slug'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('users', function (Blueprint $table) { + $table->dropUnique('users_slug_unique'); + }); + } +} diff --git a/public/templates/account/settings.html b/public/templates/account/settings.html index 3cf4b49c..e345027e 100644 --- a/public/templates/account/settings.html +++ b/public/templates/account/settings.html @@ -9,19 +9,27 @@
- - -
Your current MLP Forums display name is {{settings.mlpforums_name}}
+ +
{{errors.display_name}}
+ +
+ + +
{{errors.slug}}
+
+
+
{{errors.description}}
+