Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-04-01 02:41:39 -05:00
Code Issues Activity

Merge pull request #4220 from felixrieseberg/iss4211

Browse source 
Shorter user slugs (if possible)
This commit is contained in:
Hannah Wolfe 2014-10-16 21:23:50 +03:00
commit d44a97405b
7 changed files with 107 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
core/server/models/base.js
View file

@ -316,7 +316,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
slugTryCount = 1,
baseName = Model.prototype.tableName.replace(/s$/, ''),
// Look for a matching slug, append an incrementing number if so
checkIfSlugExists;
checkIfSlugExists, longSlug;
checkIfSlugExists = function (slugToFind) {
var args = {slug: slugToFind};
@ -333,6 +333,14 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
slugTryCount += 1;
// If we shortened, go back to the full version and try again
if (slugTryCount === 2 && longSlug) {
slugToFind = longSlug;
longSlug = null;
slugTryCount = 1;
return checkIfSlugExists(slugToFind);
}
// If this is the first time through, add the hyphen
if (slugTryCount === 2) {
slugToFind += '-';
@ -353,6 +361,12 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
// Remove trailing hyphen
slug = slug.charAt(slug.length - 1) === '-' ? slug.substr(0, slug.length - 1) : slug;
// If it's a user, let's try to cut it down (unless this is a human request)
if (baseName === 'user' && options && options.shortSlug && slugTryCount === 1 && slug !== 'ghost-owner') {
longSlug = slug;
slug = (slug.indexOf('-') > -1) ? slug.substr(0, slug.indexOf('-')) : slug;
}
// Check the filtered slug doesn't match any of the reserved keywords
return filters.doFilter('slug.reservedSlugs', config.slugs.reserved).then(function (slugList) {
// Some keywords cannot be changed

3
core/server/models/user.js
View file

@ -52,7 +52,7 @@ User = ghostBookshelf.Model.extend({
if (this.hasChanged('slug') || !this.get('slug')) {
// Generating a slug requires a db call to look for conflicting slugs
return ghostBookshelf.Model.generateSlug(User, this.get('slug') || this.get('name'),
{transacting: options.transacting})
{transacting: options.transacting, shortSlug: !this.get('slug')})
.then(function (slug) {
self.set({slug: slug});
});
@ -493,6 +493,7 @@ User = ghostBookshelf.Model.extend({
options = this.filterOptions(options, 'setup');
options.withRelated = _.union(['roles'], options.include);
options.shortSlug = true;
return validatePasswordLength(userData.password).then(function () {
// Generate a new password hash

4
core/test/functional/base.js
View file

@ -30,7 +30,7 @@ var DEBUG = false, // TOGGLE THIS TO GET MORE SCREENSHOTS
url = 'http://' + host + (noPort ? '/' : ':' + port + '/'),
newUser = {
name: 'Test User',
slug: 'test-user',
slug: 'test',
email: email,
password: password
},
@ -90,7 +90,7 @@ screens = {
selector: '.settings-menu-users.active'
},
'settings.users.user': {
url: 'ghost/settings/users/test-user',
url: 'ghost/settings/users/test',
linkSelector: '.user-menu-profile',
selector: '.user-profile'
},

16
core/test/functional/client/settings_test.js
View file

@ -199,7 +199,7 @@ CasperTest.begin('Users screen is correct', 9, function suite(test) {
CasperTest.begin('Can save settings', 7, function suite(test) {
casper.thenOpenAndWaitForPageLoad('settings.users.user', function testTitleAndUrl() {
test.assertTitle('Ghost Admin', 'Ghost Admin title is GhostAdmin');
test.assertUrlMatch(/ghost\/settings\/users\/test-user\/$/, 'settings.users.user has correct URL');
test.assertUrlMatch(/ghost\/settings\/users\/test\/$/, 'settings.users.user has correct URL');
});
function handleUserRequest(requestData) {
@ -274,7 +274,7 @@ CasperTest.begin('User settings screen resets all whitespace slug to original va
casper.thenOpenAndWaitForPageLoad('settings.users.user', function testTitleAndUrl() {
test.assertTitle('Ghost Admin', 'Ghost admin has no title');
test.assertUrlMatch(/ghost\/settings\/users\/test-user\/$/, 'Ghost doesn\'t require login this time');
test.assertUrlMatch(/ghost\/settings\/users\/test\/$/, 'Ghost doesn\'t require login this time');
});
casper.then(function setSlugToAllWhitespace() {
@ -298,7 +298,7 @@ CasperTest.begin('User settings screen change slug handles duplicate slug', 4, f
casper.thenOpenAndWaitForPageLoad('settings.users.user', function testTitleAndUrl() {
test.assertTitle('Ghost Admin', 'Ghost admin has no title');
test.assertUrlMatch(/ghost\/settings\/users\/test-user\/$/, 'Ghost doesn\'t require login this time');
test.assertUrlMatch(/ghost\/settings\/users\/test\/$/, 'Ghost doesn\'t require login this time');
});
casper.then(function changeSlug() {
@ -325,7 +325,7 @@ CasperTest.begin('User settings screen validates email', 6, function suite(test)
casper.thenOpenAndWaitForPageLoad('settings.users.user', function testTitleAndUrl() {
test.assertTitle('Ghost Admin', 'Ghost admin has no title');
test.assertUrlMatch(/ghost\/settings\/users\/test-user\/$/, 'Ghost doesn\'t require login this time');
test.assertUrlMatch(/ghost\/settings\/users\/test\/$/, 'Ghost doesn\'t require login this time');
});
casper.then(function setEmailToInvalid() {
@ -366,7 +366,7 @@ CasperTest.begin('User settings screen validates email', 6, function suite(test)
CasperTest.begin('User settings screen shows remaining characters for Bio properly', 4, function suite(test) {
casper.thenOpenAndWaitForPageLoad('settings.users.user', function testTitleAndUrl() {
test.assertTitle('Ghost Admin', 'Ghost admin has no title');
test.assertUrlMatch(/ghost\/settings\/users\/test-user\/$/, 'Ghost doesn\'t require login this time');
test.assertUrlMatch(/ghost\/settings\/users\/test\/$/, 'Ghost doesn\'t require login this time');
});
function getRemainingBioCharacterCount() {
@ -391,7 +391,7 @@ CasperTest.begin('User settings screen shows remaining characters for Bio proper
CasperTest.begin('Ensure user bio field length validation', 3, function suite(test) {
casper.thenOpenAndWaitForPageLoad('settings.users.user', function testTitleAndUrl() {
test.assertTitle('Ghost Admin', 'Ghost admin has no title');
test.assertUrlMatch(/ghost\/settings\/users\/test-user\/$/, 'Ghost doesn\'t require login this time');
test.assertUrlMatch(/ghost\/settings\/users\/test\/$/, 'Ghost doesn\'t require login this time');
});
casper.then(function setBioToInvalid() {
@ -410,7 +410,7 @@ CasperTest.begin('Ensure user bio field length validation', 3, function suite(te
CasperTest.begin('Ensure user url field validation', 3, function suite(test) {
casper.thenOpenAndWaitForPageLoad('settings.users.user', function testTitleAndUrl() {
test.assertTitle('Ghost Admin', 'Ghost admin has no title');
test.assertUrlMatch(/ghost\/settings\/users\/test-user\/$/, 'Ghost doesn\'t require login this time');
test.assertUrlMatch(/ghost\/settings\/users\/test\/$/, 'Ghost doesn\'t require login this time');
});
casper.then(function setWebsiteToInvalid() {
@ -429,7 +429,7 @@ CasperTest.begin('Ensure user url field validation', 3, function suite(test) {
CasperTest.begin('Ensure user location field length validation', 3, function suite(test) {
casper.thenOpenAndWaitForPageLoad('settings.users.user', function testTitleAndUrl() {
test.assertTitle('Ghost Admin', 'Ghost admin has no title');
test.assertUrlMatch(/ghost\/settings\/users\/test-user\/$/, 'Ghost doesn\'t require login this time');
test.assertUrlMatch(/ghost\/settings\/users\/test\/$/, 'Ghost doesn\'t require login this time');
});
casper.then(function setLocationToInvalid() {

2
core/test/integration/api/api_slugs_spec.js
View file

@ -39,7 +39,7 @@ describe('Slug API', function () {
});
it('can generate user slug', function (done) {
SlugAPI.generate({context: {user: 1}, type: 'tag', name: 'user name'})
SlugAPI.generate({context: {user: 1}, type: 'user', name: 'user name'})
.then(function (results) {
should.exist(results);
testUtils.API.checkResponse(results, 'slugs');

35
core/test/integration/model/model_tags_spec.js
View file

@ -329,6 +329,41 @@ describe('Tag Model', function () {
}).catch(done);
});
it('correctly creates non-conflicting slugs', function (done) {
var seededTagNames = ['tag1'],
postModel;
seedTags(seededTagNames).then(function (_postModel) {
postModel = _postModel;
return TagModel.add({name: 'tagc'}, context);
}).then(function () {
// the tag API expects tags to be provided like {id: 1, name: 'draft'}
var tagData = [];
tagData.push({id: 2, name: 'tagc'});
tagData.push({id: 3, name: 'tagc++'});
postModel = postModel.toJSON();
postModel.tags = tagData;
return PostModel.edit(postModel, _.extend(context, {id: postModel.id, withRelated: ['tags']}));
}).then(function () {
return PostModel.findOne({id: postModel.id, status: 'all'}, {withRelated: ['tags']});
}).then(function (reloadedPost) {
var tagModels = reloadedPost.related('tags').models,
tagSlugs = tagModels.map(function (t) { return t.get('slug'); }),
tagIds = _.pluck(tagModels, 'id');
tagSlugs.sort().should.eql(['tagc', 'tagc-2']);
// make sure it hasn't just added a new tag with the same name
// Don't expect a certain order in results - check for number of items!
Math.max.apply(Math, tagIds).should.eql(3);
done();
}).catch(done);
});
it('can add a tag to a post on creation', function (done) {
var newPost = _.extend(testUtils.DataGenerator.forModel.posts[0], {tags: [{name: 'test_tag_1'}]});

44
core/test/integration/model/model_users_spec.js
View file

@ -45,6 +45,50 @@ describe('User Model', function run() {
}).catch(done);
});
it('shortens slug if possible', function (done) {
var userData = testUtils.DataGenerator.forModel.users[2];
sandbox.stub(UserModel, 'gravatarLookup', function (userData) {
return Promise.resolve(userData);
});
UserModel.add(userData, context).then(function (createdUser) {
should.exist(createdUser);
createdUser.has('slug').should.equal(true);
createdUser.attributes.slug.should.equal('jimothy');
done();
}).catch(done);
});
it('does not short slug if not possible', function (done) {
var userData = testUtils.DataGenerator.forModel.users[2];
sandbox.stub(UserModel, 'gravatarLookup', function (userData) {
return Promise.resolve(userData);
});
UserModel.add(userData, context).then(function (createdUser) {
should.exist(createdUser);
createdUser.has('slug').should.equal(true);
createdUser.attributes.slug.should.equal('jimothy');
}).then(function () {
userData.email = 'newmail@mail.com';
UserModel.add(userData, context).then(function (createdUser) {
should.exist(createdUser);
createdUser.has('slug').should.equal(true);
createdUser.attributes.slug.should.equal('jimothy-bogendath');
}).then(function () {
userData.email = 'newmail2@mail.com';
UserModel.add(userData, context).then(function (createdUser) {
should.exist(createdUser);
createdUser.has('slug').should.equal(true);
createdUser.attributes.slug.should.equal('jimothy-bogendath-2');
done();
});
});
}).catch(done);
});
it('does NOT lowercase email', function (done) {
var userData = testUtils.DataGenerator.forModel.users[2];