Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00
Code Issues Activity

Correct behavior on updating user slug.

Browse source 
No Issue
- Defer save until after slug is checked.
- If new slug is empty or all whitespace, reset to previous value.
- If new slug is the same as existing slug except for an increment
  value (e.g. ghost-user-2), use the original slug.
- If the slug has changed, change the URL path to reflect the
  change so that the browser refresh and back button still work.
- Added tests.
This commit is contained in:
Jason Williams 2014-08-12 18:56:31 +00:00
parent 868219d0a9
commit 1c4c4eb5de
3 changed files with 124 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

105
core/client/controllers/settings/users/user.js
View file

@ -2,16 +2,14 @@ import SlugGenerator from 'ghost/models/slug-generator';
var SettingsUserController = Ember.ObjectController.extend({
_lastSlug: null,
updateLastSlug: Ember.observer(function () {
this.set('_lastSlug', this.get('user.slug'));
}),
user: Ember.computed.alias('model'),
email: Ember.computed.readOnly('user.email'),
slugValue: Ember.computed.oneWay('user.slug'),
lastPromise: null,
coverDefault: function () {
return this.get('ghostPaths.url').asset('/shared/img/user-cover.png');
}.property('ghostPaths'),
@ -106,15 +104,43 @@ var SettingsUserController = Ember.ObjectController.extend({
save: function () {
var user = this.get('user'),
slugValue = this.get('slugValue'),
afterUpdateSlug = this.get('lastPromise'),
promise,
slugChanged,
self = this;
user.save({ format: false }).then(function (model) {
if (user.get('slug') !== slugValue) {
slugChanged = true;
user.set('slug', slugValue);
}
promise = Ember.RSVP.resolve(afterUpdateSlug).then(function () {
return user.save({ format: false });
}).then(function (model) {
var currentPath,
newPath;
self.notifications.showSuccess('Settings successfully saved.');
// If the user's slug has changed, change the URL and replace
// the history so refresh and back button still work
if (slugChanged) {
currentPath = window.history.state.path;
newPath = currentPath.split('/');
newPath[newPath.length - 2] = model.get('slug');
newPath = newPath.join('/');
window.history.replaceState({ path: newPath }, '', newPath);
}
return model;
}).catch(function (errors) {
self.notifications.showErrors(errors);
});
this.set('lastPromise', promise);
},
password: function () {
@ -143,46 +169,57 @@ var SettingsUserController = Ember.ObjectController.extend({
},
updateSlug: function (newSlug) {
var slug = this.get('_lastSlug'),
self = this;
var self = this,
afterSave = this.get('lastPromise'),
promise;
newSlug = newSlug || slug;
promise = Ember.RSVP.resolve(afterSave).then(function () {
var slug = self.get('slug');
newSlug = newSlug.trim();
newSlug = newSlug || slug;
// Ignore unchanged slugs or candidate slugs that are empty
if (!newSlug || slug === newSlug) {
return;
}
newSlug = newSlug.trim();
this.get('slugGenerator').generateSlug(newSlug).then(function (serverSlug) {
// Ignore unchanged slugs or candidate slugs that are empty
if (!newSlug || slug === newSlug) {
self.set('slugValue', slug);
// If after getting the sanitized and unique slug back from the API
// we end up with a slug that matches the existing slug, abort the change
if (serverSlug === slug) {
return;
}
// Because the server transforms the candidate slug by stripping
// certain characters and appending a number onto the end of slugs
// to enforce uniqueness, there are cases where we can get back a
// candidate slug that is a duplicate of the original except for
// the trailing incrementor (e.g., this-is-a-slug and this-is-a-slug-2)
return self.get('slugGenerator').generateSlug(newSlug).then(function (serverSlug) {
// get the last token out of the slug candidate and see if it's a number
var slugTokens = serverSlug.split('-'),
check = Number(slugTokens.pop());
// if the candidate slug is the same as the existing slug except
// for the incrementor then the existing slug should be used
if (_.isNumber(check) && check > 0) {
if (slug === slugTokens.join('-') && serverSlug !== newSlug) {
// If after getting the sanitized and unique slug back from the API
// we end up with a slug that matches the existing slug, abort the change
if (serverSlug === slug) {
return;
}
}
self.set('_lastSlug', serverSlug);
// Because the server transforms the candidate slug by stripping
// certain characters and appending a number onto the end of slugs
// to enforce uniqueness, there are cases where we can get back a
// candidate slug that is a duplicate of the original except for
// the trailing incrementor (e.g., this-is-a-slug and this-is-a-slug-2)
// get the last token out of the slug candidate and see if it's a number
var slugTokens = serverSlug.split('-'),
check = Number(slugTokens.pop());
// if the candidate slug is the same as the existing slug except
// for the incrementor then the existing slug should be used
if (_.isNumber(check) && check > 0) {
if (slug === slugTokens.join('-') && serverSlug !== newSlug) {
self.set('slugValue', slug);
return;
}
}
self.set('slugValue', serverSlug);
});
});
this.set('lastPromise', promise);
}
}
});

5
core/client/templates/settings/users/user.hbs
View file

@ -60,9 +60,8 @@
<div class="form-group">
<label for="user-slug">Slug</label>
{{!-- {{input value=user.slug id="user-slug" placeholder="Slug" autocorrect="off"}} --}}
{{gh-blur-input class="user-name" id="user-slug" value=user.slug name="user" action="updateSlug" placeholder="Slug" selectOnClick="true" autocorrect="off"}}
<p>{{gh-blog-url}}/author/{{user.slug}}</p>
{{gh-blur-input class="user-name" id="user-slug" value=slugValue name="user" action="updateSlug" placeholder="Slug" selectOnClick="true" autocorrect="off"}}
<p>{{gh-blog-url}}/author/{{slugValue}}</p>
</div>
<div class="form-group">

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

@ -293,6 +293,57 @@ CasperTest.begin('Can save settings', 7, function suite(test) {
});
});
CasperTest.begin('User settings screen resets all whitespace slug to original value', 3, function suite(test) {
var slug;
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');
});
casper.then(function setSlugToAllWhitespace() {
slug = casper.getElementInfo('#user-slug').attributes.value;
casper.fillSelectors('.user-profile', {
'#user-slug': ' '
}, false);
});
casper.thenClick('.content.settings-user');
casper.then(function checkSlugInputValue() {
casper.wait(250);
test.assertField('user', slug);
});
});
CasperTest.begin('User settings screen change slug handles duplicate slug', 4, function suite(test) {
var slug;
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');
});
casper.then(function changeSlug() {
slug = casper.getElementInfo('#user-slug').attributes.value;
casper.fillSelectors('.user-profile', {
'#user-slug': slug + '!'
}, false);
});
casper.thenClick('.content.settings-user');
casper.waitForResource(/\/slugs\/user\//, function testGoodResponse(resource) {
test.assert(400 > resource.status);
});
casper.then(function checkSlugInputValue() {
test.assertField('user', slug);
});
});
CasperTest.begin('User settings screen validates email', 6, function suite(test) {
var email, brokenEmail;