Fixes #51: Slugs are now unique and user-changeable.

This commit is contained in:
Peter Deltchev 2016-06-05 20:05:51 -07:00
parent 9d0229d1f6
commit b46ba956d0
12 changed files with 327 additions and 26 deletions

View file

@ -37,6 +37,8 @@ class SaveAccountSettingsCommand extends CommandBase
{
$this->_input = $input;
$this->_slug = $slug;
/** @var User _user */
$this->_user = null;
$this->_current = null;
}
@ -83,13 +85,16 @@ class SaveAccountSettingsCommand extends CommandBase
$rules = [
'display_name' => 'required|min:3|max:26',
'bio' => 'textarea_length:250'
'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';

View file

@ -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,

View file

@ -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+');

View file

@ -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);
}
}

View file

@ -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)

View file

@ -0,0 +1,94 @@
<?php
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Database\Migrations\Migration;
class EnforceUniqueSlugs extends Migration
{
/**
* Run the migrations.
*
* @return void
*/
public function up()
{
// Fix slugs retroactively
$slugs = DB::table('users')
->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');
});
}
}

View file

@ -9,19 +9,27 @@
</ul>
<div class="stretch-to-bottom">
<div class="form-row" ng-class="{'has-error': errors.display_name != null}">
<label for="sync_names" class="strong"><input ng-disabled="isSaving" ng-change="touchModel();" id="sync_names" type="checkbox" ng-model="settings.sync_names" /> Sync my MLP Forums display name with Pony.fm</label>
<input type="text" ng-disabled="isSaving" ng-change="touchModel()" ng-show="!settings.sync_names" placeholder="Display Name" id="display_name" ng-model="settings.display_name" />
<div ng-show="settings.sync_names" class="alert alert-info">Your current MLP Forums display name is <strong>{{settings.mlpforums_name}}</strong></div>
<label class="strong" for="display_name">Display Name</label>
<input type="text" ng-disabled="isSaving" ng-change="touchModel()" placeholder="Display Name" id="display_name" ng-model="settings.display_name" />
<div class="error">{{errors.display_name}}</div>
</div>
<div class="form-row" ng-class="{'has-error': errors.slug != null}">
<label class="strong" for="slug">Slug (your profile URL: https://pony.fm/{{settings.slug}})</label>
<input type="text" ng-disabled="isSaving" ng-change="touchModel()" placeholder="slug" id="slug" ng-model="settings.slug" />
<div class="error">{{errors.slug}}</div>
</div>
<div class="form-row">
<label for="can_see_explicit_content" class="strong"><input ng-change="touchModel()" ng-disabled="isLoading" id="can_see_explicit_content" type="checkbox" ng-model="settings.can_see_explicit_content" /> Can See Explicit Content</label>
</div>
<div class="form-row" ng-class="{'has-error': errors.bio != null}">
<label class="strong" for="bio">Bio</label>
<textarea id="bio" placeholder="bio (optional)" ng-model="settings.bio" ng-disabled="isLoading" ng-change="touchModel()"></textarea>
<div class="error">{{errors.description}}</div>
</div>
<div class="form-row" ng-class="{'has-error': errors.avatar != null || errors.gravatar != null}">
<label for="uses_gravatar" class="strong">
<input ng-change="touchModel()" ng-disabled="isLoading" id="uses_gravatar" type="checkbox" ng-model="settings.uses_gravatar" /> Use Gravatar

View file

@ -1,12 +1,12 @@
<div class="resource-details artist-details" ng-class="::{'x-archived': artist.is_archived}">
<header ng-style="{'background-image': 'linear-gradient(135deg, ' + artist.avatar_colors[0] + ' 15%, ' + artist.avatar_colors[1] + ' 100%)'}">
<img src="{{::artist.avatars.normal}}">
<img src="{{artist.avatars.normal}}">
<div class="artist-right">
<h1>{{::artist.name}}<i class="fa fa-star admin-star" ng-show="::artist.isAdmin" data-title="Admin" bs-tooltip></i></h1>
<a href="#" class="btn btn-default" ng-class="{'btn-primary': !artist.user_data.is_following}" ng-show="::auth.isLogged && auth.user.id != artist.id" pfm-eat-click ng-click="toggleFollow()">
<span ng-hide="artist.user_data.is_following">Follow</span>
<span ng-show="artist.user_data.is_following">Following!</span>
<h1>{{artist.name}}<i class="fa fa-star admin-star" ng-show="::artist.isAdmin" data-title="Admin" bs-tooltip></i></h1>
<a href="#" class="btn btn-default" ng-class="{'btn-primary': !artist.user_data.is_following}" ng-show="auth.isLogged && auth.user.id != artist.id" pfm-eat-click ng-click="toggleFollow()">
<span ng-if="!artist.user_data.is_following">Follow</span>
<span ng-if="artist.user_data.is_following">Following!</span>
</a>
</div>

View file

@ -24,11 +24,29 @@ module.exports = angular.module('ponyfm').controller "account-settings", [
$scope.touchModel = () ->
$scope.isDirty = true
$scope.refresh = () ->
$.getJSON('/api/web/account/settings/' + $state.params.slug)
$scope.updateUrl = (newSlug) ->
$state.go(
'content.artist.account.settings',
{slug: newSlug},
{location: 'replace', reload: true}
).then ->
$scope.$emit('user-updated')
$scope.refresh = (refreshSlug = false) ->
currentSlug = if refreshSlug then $scope.settings.slug else $state.params.slug
auth.refresh()
$.getJSON('/api/web/account/settings/' + currentSlug)
.done (res) -> $scope.$apply ->
$scope.settings = res
if refreshSlug
# Ensures the slug in the URL is up to date - the "content.artist"
# state depends on it to fetch updated profile data.
$scope.updateUrl(currentSlug)
$scope.setAvatar = (image, type) ->
delete $scope.settings.avatar_id
delete $scope.settings.avatar
@ -49,12 +67,12 @@ module.exports = angular.module('ponyfm').controller "account-settings", [
response = $.parseJSON(xhr.responseText)
if xhr.status != 200
$scope.errors = {}
_.each response.errors, (value, key) -> $scope.errors[key] = value.join ', '
_.each response.errors, (value, key) -> $scope.errors[key] = value.join ' '
return
$scope.isDirty = false
$scope.errors = {}
$scope.refresh()
$scope.refresh(true)
formData = new FormData()

View file

@ -23,11 +23,17 @@ window.pfm.preloaders['artist'] = [
module.exports = angular.module('ponyfm').controller "artist", [
'$scope', 'artists', '$state', 'follow'
($scope, artists, $state, follow) ->
artists.fetch($state.params.slug)
updateArtist = (force = false) ->
artists.fetch($state.params.slug, force)
.done (artistResponse) ->
$scope.artist = artistResponse.artist
$scope.toggleFollow = () ->
follow.toggle('artist', $scope.artist.id).then (res) ->
$scope.artist.user_data.is_following = res.is_followed
updateArtist()
$scope.$on 'user-updated', ->
updateArtist(true)
]

View file

@ -30,4 +30,17 @@ module.exports = angular.module('ponyfm').factory('auth', [
def.promise()
logout: -> $.post('/api/web/auth/logout')
# Updates the pfm.auth.user object's values with the current server-side ones.
refresh: ->
def = new $.Deferred()
$.get("/api/web/users/#{window.pfm.auth.user.id}")
.done (data) ->
_.extend(window.pfm.auth.user, data.user)
$rootScope.$apply -> def.resolve()
.fail (response) ->
$rootScope.$apply -> def.reject response.responseJSON
def.promise()
])

View file

@ -100,6 +100,7 @@ return [
"min_width" => "The :attribute is not wide enough.",
"min_height" => "The :attribute is not tall enough.",
"textarea_length" => "The :attribute must be less than 250 characters long.", // @TODO: Figure out how to retrieve the parameter from the validation rule
'is_not_reserved_slug' => 'This :attribute is reserved. Please pick another one.',
/*
|--------------------------------------------------------------------------