From e96b60b850e1416daf920003c23297ef2f7073f0 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 17 May 2016 14:06:12 +0100 Subject: [PATCH 1/4] Add helpers for facebook & twitter urls refs #6534 - this PR assumes that we are now saving usernames only in the database for twitter & facebook - adds a new social links utility which can generate twitter & facebook urls from the username - adds a {{twitter_url}} and {{facebook_url}} helper which uses these - adds a full suite of tests for the helpers & utils --- core/server/helpers/facebook_url.js | 26 ++++++++++ core/server/helpers/index.js | 4 ++ core/server/helpers/twitter_url.js | 26 ++++++++++ core/server/helpers/utils.js | 14 +++++- core/server/utils/social-urls.js | 9 ++++ .../unit/server_helpers/facebook_url_spec.js | 48 +++++++++++++++++++ .../unit/server_helpers/twitter_url_spec.js | 48 +++++++++++++++++++ core/test/unit/social-urls_spec.js | 39 +++++++++++++++ 8 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 core/server/helpers/facebook_url.js create mode 100644 core/server/helpers/twitter_url.js create mode 100644 core/server/utils/social-urls.js create mode 100644 core/test/unit/server_helpers/facebook_url_spec.js create mode 100644 core/test/unit/server_helpers/twitter_url_spec.js create mode 100644 core/test/unit/social-urls_spec.js diff --git a/core/server/helpers/facebook_url.js b/core/server/helpers/facebook_url.js new file mode 100644 index 00000000000..d3030c17e51 --- /dev/null +++ b/core/server/helpers/facebook_url.js @@ -0,0 +1,26 @@ +// # Facebook URL Helper +// Usage: `{{facebook_url}}` or `{{facebook_url author.facebook}}` +// +// Output a url for a twitter username +// +// We use the name facebook_url to match the helper for consistency: +// jscs:disable requireCamelCaseOrUpperCaseIdentifiers + +var socialUrls = require('../utils/social-urls'), + findKey = require('./utils').findKey, + facebook_url; + +facebook_url = function (username, options) { + if (!options) { + options = username; + username = findKey('facebook', this, options.data.blog); + } + + if (username) { + return socialUrls.facebookUrl(username); + } + + return null; +}; + +module.exports = facebook_url; diff --git a/core/server/helpers/index.js b/core/server/helpers/index.js index f806558a3a6..0783b05d1e3 100644 --- a/core/server/helpers/index.js +++ b/core/server/helpers/index.js @@ -17,6 +17,7 @@ coreHelpers.content = require('./content'); coreHelpers.date = require('./date'); coreHelpers.encode = require('./encode'); coreHelpers.excerpt = require('./excerpt'); +coreHelpers.facebook_url = require('./facebook_url'); coreHelpers.foreach = require('./foreach'); coreHelpers.get = require('./get'); coreHelpers.ghost_foot = require('./ghost_foot'); @@ -34,6 +35,7 @@ coreHelpers.prev_post = require('./prev_next'); coreHelpers.next_post = require('./prev_next'); coreHelpers.tags = require('./tags'); coreHelpers.title = require('./title'); +coreHelpers.twitter_url = require('./twitter_url'); coreHelpers.url = require('./url'); // Specialist helpers for certain templates @@ -110,6 +112,8 @@ registerHelpers = function (adminHbs) { registerThemeHelper('post_class', coreHelpers.post_class); registerThemeHelper('tags', coreHelpers.tags); registerThemeHelper('title', coreHelpers.title); + registerThemeHelper('twitter_url', coreHelpers.twitter_url); + registerThemeHelper('facebook_url', coreHelpers.facebook_url); registerThemeHelper('url', coreHelpers.url); // Async theme helpers diff --git a/core/server/helpers/twitter_url.js b/core/server/helpers/twitter_url.js new file mode 100644 index 00000000000..38b381e51ac --- /dev/null +++ b/core/server/helpers/twitter_url.js @@ -0,0 +1,26 @@ +// # Twitter URL Helper +// Usage: `{{twitter_url}}` or `{{twitter_url author.twitter}}` +// +// Output a url for a twitter username +// +// We use the name twitter_url to match the helper for consistency: +// jscs:disable requireCamelCaseOrUpperCaseIdentifiers + +var socialUrls = require('../utils/social-urls'), + findKey = require('./utils').findKey, + twitter_url; + +twitter_url = function twitter_url(username, options) { + if (!options) { + options = username; + username = findKey('twitter', this, options.data.blog); + } + + if (username) { + return socialUrls.twitterUrl(username); + } + + return null; +}; + +module.exports = twitter_url; diff --git a/core/server/helpers/utils.js b/core/server/helpers/utils.js index 2d828a20733..e51ea905fef 100644 --- a/core/server/helpers/utils.js +++ b/core/server/helpers/utils.js @@ -6,7 +6,19 @@ utils = { linkTemplate: _.template('<%= text %>'), scriptTemplate: _.template(''), inputTemplate: _.template(' />'), - isProduction: process.env.NODE_ENV === 'production' + isProduction: process.env.NODE_ENV === 'production', + // @TODO this can probably be made more generic and used in more places + findKey: function findKey(key, object, data) { + if (object && _.has(object, key) && !_.isEmpty(object[key])) { + return object[key]; + } + + if (data && _.has(data, key) && !_.isEmpty(data[key])) { + return data[key]; + } + + return null; + } }; module.exports = utils; diff --git a/core/server/utils/social-urls.js b/core/server/utils/social-urls.js new file mode 100644 index 00000000000..fffe1fbcc5b --- /dev/null +++ b/core/server/utils/social-urls.js @@ -0,0 +1,9 @@ +module.exports.twitterUrl = function twitterUrl(username) { + // Creates the canonical twitter URL without the '@' + return 'https://twitter.com/' + username.replace(/^@/, ''); +}; + +module.exports.facebookUrl = function facebookUrl(username) { + // Handles a starting slash, this shouldn't happen, but just in case + return 'https://www.facebook.com/' + username.replace(/^\//, ''); +}; diff --git a/core/test/unit/server_helpers/facebook_url_spec.js b/core/test/unit/server_helpers/facebook_url_spec.js new file mode 100644 index 00000000000..cce71c759b9 --- /dev/null +++ b/core/test/unit/server_helpers/facebook_url_spec.js @@ -0,0 +1,48 @@ +/*globals describe, before, beforeEach, it*/ +var should = require('should'), + hbs = require('express-hbs'), + utils = require('./utils'), + +// Stuff we are testing + handlebars = hbs.handlebars, + helpers = require('../../../server/helpers'); + +describe('{{facebook_url}} helper', function () { + var options = {data: {blog: {}}}; + + before(function () { + utils.loadHelpers(); + }); + + beforeEach(function () { + options.data.blog = {facebook: ''}; + }); + + it('has loaded facebook_url helper', function () { + should.exist(handlebars.helpers.facebook_url); + }); + + it('should output the facebook url for @blog, if no other facebook username is provided', function () { + options.data.blog = {facebook: 'hey'}; + + helpers.facebook_url.call({}, options).should.equal('https://www.facebook.com/hey'); + }); + + it('should output the facebook url for the local object, if it has one', function () { + options.data.blog = {facebook: 'hey'}; + + helpers.facebook_url.call({facebook: 'you/there'}, options).should.equal('https://www.facebook.com/you/there'); + }); + + it('should output the facebook url for the provided username when it is explicitly passed in', function () { + options.data.blog = {facebook: 'hey'}; + + helpers.facebook_url.call({facebook: 'you/there'}, 'i/see/you/over/there', options) + .should.equal('https://www.facebook.com/i/see/you/over/there'); + }); + + it('should return null if there are no facebook usernames', function () { + should.equal(helpers.facebook_url(options), null); + }); +}); + diff --git a/core/test/unit/server_helpers/twitter_url_spec.js b/core/test/unit/server_helpers/twitter_url_spec.js new file mode 100644 index 00000000000..fc7e67f08d6 --- /dev/null +++ b/core/test/unit/server_helpers/twitter_url_spec.js @@ -0,0 +1,48 @@ +/*globals describe, before, beforeEach, it*/ +var should = require('should'), + hbs = require('express-hbs'), + utils = require('./utils'), + +// Stuff we are testing + handlebars = hbs.handlebars, + helpers = require('../../../server/helpers'); + +describe('{{twitter_url}} helper', function () { + var options = {data: {blog: {}}}; + + before(function () { + utils.loadHelpers(); + }); + + beforeEach(function () { + options.data.blog = {twitter: ''}; + }); + + it('has loaded twitter_url helper', function () { + should.exist(handlebars.helpers.twitter_url); + }); + + it('should output the twitter url for @blog, if no other twitter username is provided', function () { + options.data.blog = {twitter: '@hey'}; + + helpers.twitter_url.call({}, options).should.equal('https://twitter.com/hey'); + }); + + it('should output the twitter url for the local object, if it has one', function () { + options.data.blog = {twitter: '@hey'}; + + helpers.twitter_url.call({twitter: '@youthere'}, options).should.equal('https://twitter.com/youthere'); + }); + + it('should output the twitter url for the provided username when it is explicitly passed in', function () { + options.data.blog = {twitter: '@hey'}; + + helpers.twitter_url.call({twitter: '@youthere'}, '@iseeyouoverthere', options) + .should.equal('https://twitter.com/iseeyouoverthere'); + }); + + it('should return null if there are no twitter usernames', function () { + should.equal(helpers.twitter_url(options), null); + }); +}); + diff --git a/core/test/unit/social-urls_spec.js b/core/test/unit/social-urls_spec.js new file mode 100644 index 00000000000..93fe0d8fbe4 --- /dev/null +++ b/core/test/unit/social-urls_spec.js @@ -0,0 +1,39 @@ +/*globals describe, it*/ +var should = require('should'), + + // Stuff we are testing + socialUrls = require('../../server/utils/social-urls'); + +describe('Social Urls', function () { + it('should have a twitter url function', function () { + should.exist(socialUrls.twitterUrl); + }); + + it('should have a facebook url function', function () { + should.exist(socialUrls.facebookUrl); + }); + + describe('twitter', function () { + it('should return a correct concatenated URL', function () { + socialUrls.twitterUrl('myusername').should.eql('https://twitter.com/myusername'); + }); + + it('should return a url without an @ sign if one is provided', function () { + socialUrls.twitterUrl('@myusername').should.eql('https://twitter.com/myusername'); + }); + }); + + describe('facebook', function () { + it('should return a correct concatenated URL', function () { + socialUrls.facebookUrl('myusername').should.eql('https://www.facebook.com/myusername'); + }); + + it('should return a correct concatenated URL for usernames with slashes', function () { + socialUrls.facebookUrl('page/xxx/123').should.eql('https://www.facebook.com/page/xxx/123'); + }); + + it('should return a correct concatenated URL for usernames which start with a slash', function () { + socialUrls.facebookUrl('/page/xxx/123').should.eql('https://www.facebook.com/page/xxx/123'); + }); + }); +}); From 29c9e8b1475e72a29df3c5a79c75ee40426ac370 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 17 May 2016 15:06:09 +0100 Subject: [PATCH 2/4] Update structured data for fb & twitter usernames refs #6534 - twitter & facebook fields are changing to store usernames only - use the new social url util to generate urls where necessary - update tests --- core/server/data/meta/schema.js | 9 +++--- core/server/data/meta/structured_data.js | 15 ++++------ core/test/unit/metadata/schema_spec.js | 8 +++--- .../unit/metadata/structured_data_spec.js | 6 ++-- .../unit/server_helpers/ghost_head_spec.js | 28 +++++++++---------- 5 files changed, 32 insertions(+), 34 deletions(-) diff --git a/core/server/data/meta/schema.js b/core/server/data/meta/schema.js index a4ad4169fbd..ad4eee23956 100644 --- a/core/server/data/meta/schema.js +++ b/core/server/data/meta/schema.js @@ -1,5 +1,6 @@ var config = require('../../config'), hbs = require('express-hbs'), + socialUrls = require('../../utils/social-urls'), escapeExpression = hbs.handlebars.Utils.escapeExpression, _ = require('lodash'); @@ -23,20 +24,20 @@ function trimSameAs(data, context) { sameAs.push(data.post.author.website); } if (data.post.author.facebook) { - sameAs.push(data.post.author.facebook); + sameAs.push(socialUrls.facebookUrl(data.post.author.facebook)); } if (data.post.author.twitter) { - sameAs.push(data.post.author.twitter); + sameAs.push(socialUrls.twitterUrl(data.post.author.twitter)); } } else if (context === 'author') { if (data.author.website) { sameAs.push(data.author.website); } if (data.author.facebook) { - sameAs.push(data.author.facebook); + sameAs.push(socialUrls.facebookUrl(data.author.facebook)); } if (data.author.twitter) { - sameAs.push(data.author.twitter); + sameAs.push(socialUrls.twitterUrl(data.author.twitter)); } } diff --git a/core/server/data/meta/structured_data.js b/core/server/data/meta/structured_data.js index f0c9db6fd9d..13cfe131237 100644 --- a/core/server/data/meta/structured_data.js +++ b/core/server/data/meta/structured_data.js @@ -1,16 +1,13 @@ +var socialUrls = require('../../utils/social-urls'); + function getStructuredData(metaData) { var structuredData, - card = 'summary', - twitterUser; + card = 'summary'; if (metaData.coverImage) { card = 'summary_large_image'; } - if (metaData.creatorTwitter) { - twitterUser = '@' + metaData.creatorTwitter.match(/(?:https:\/\/)(?:twitter\.com)\/(?:#!\/)?@?([^\/]*)/)[1]; - } - structuredData = { 'og:site_name': metaData.blog.title, 'og:type': metaData.ogType, @@ -21,8 +18,8 @@ function getStructuredData(metaData) { 'article:published_time': metaData.publishedDate, 'article:modified_time': metaData.modifiedDate, 'article:tag': metaData.keywords, - 'article:publisher': metaData.blog.facebook || undefined, - 'article:author': metaData.authorFacebook || undefined, + 'article:publisher': metaData.blog.facebook ? socialUrls.facebookUrl(metaData.blog.facebook) : undefined, + 'article:author': metaData.authorFacebook ? socialUrls.facebookUrl(metaData.authorFacebook) : undefined, 'twitter:card': card, 'twitter:title': metaData.metaTitle, 'twitter:description': metaData.metaDescription || metaData.excerpt, @@ -33,7 +30,7 @@ function getStructuredData(metaData) { 'twitter:label2': metaData.keywords ? 'Filed under' : undefined, 'twitter:data2': metaData.keywords ? metaData.keywords.join(', ') : undefined, 'twitter:site': metaData.blog.twitter || undefined, - 'twitter:creator': twitterUser || undefined + 'twitter:creator': metaData.creatorTwitter || undefined }; // return structured data removing null or undefined keys diff --git a/core/test/unit/metadata/schema_spec.js b/core/test/unit/metadata/schema_spec.js index 2e9299e8dd7..932c1bda3dd 100644 --- a/core/test/unit/metadata/schema_spec.js +++ b/core/test/unit/metadata/schema_spec.js @@ -10,7 +10,7 @@ describe('getSchema', function () { logo: 'http://mysite.com/author/image/url/logo.jpg' }, authorImage: 'http://mysite.com/author/image/url/me.jpg', - authorFacebook: 'https://facebook.com/testuser', + authorFacebook: 'testuser', creatorTwitter: '@testuser', authorUrl: 'http://mysite.com/author/me/', metaTitle: 'Post Title', @@ -27,8 +27,8 @@ describe('getSchema', function () { name: 'Post Author', website: 'http://myblogsite.com/', bio: 'My author bio.', - facebook: 'https://www.facebook.com/testuser', - twitter: 'https://twitter.com/testuser' + facebook: 'testuser', + twitter: '@testuser' } } }, schema = getSchema(metadata, data); @@ -175,7 +175,7 @@ describe('getSchema', function () { author: { name: 'Author Name', website: 'http://myblogsite.com/', - twitter: 'https://twitter.com/testuser' + twitter: '@testuser' } }, schema = getSchema(metadata, data); diff --git a/core/test/unit/metadata/structured_data_spec.js b/core/test/unit/metadata/structured_data_spec.js index 13b761cfe5a..f6278b914f6 100644 --- a/core/test/unit/metadata/structured_data_spec.js +++ b/core/test/unit/metadata/structured_data_spec.js @@ -7,7 +7,7 @@ describe('getStructuredData', function () { var metadata = { blog: { title: 'Blog Title', - facebook: 'https://www.facebook.com/testuser', + facebook: 'testuser', twitter: '@testuser' }, authorName: 'Test User', @@ -17,8 +17,8 @@ describe('getStructuredData', function () { publishedDate: '2015-12-25T05:35:01.234Z', modifiedDate: '2016-01-21T22:13:05.412Z', coverImage: 'http://mysite.com/content/image/mypostcoverimage.jpg', - authorFacebook: 'https://www.facebook.com/testpage', - creatorTwitter: 'https://twitter.com/twitterpage', + authorFacebook: 'testpage', + creatorTwitter: '@twitterpage', keywords: ['one', 'two', 'tag'], metaDescription: 'Post meta description' }, structuredData = getStructuredData(metadata); diff --git a/core/test/unit/server_helpers/ghost_head_spec.js b/core/test/unit/server_helpers/ghost_head_spec.js index a491b6688aa..e0116f6946a 100644 --- a/core/test/unit/server_helpers/ghost_head_spec.js +++ b/core/test/unit/server_helpers/ghost_head_spec.js @@ -124,8 +124,8 @@ describe('{{ghost_head}} helper', function () { slug: 'Author', image: '/content/images/test-author-image.png', website: 'http://authorwebsite.com', - facebook: 'https://www.facebook.com/testuser', - twitter: 'https://twitter.com/testuser', + facebook: 'testuser', + twitter: '@testuser', bio: 'Author bio' } }; @@ -301,8 +301,8 @@ describe('{{ghost_head}} helper', function () { image: '/content/images/test-author-image.png', cover: '/content/images/author-cover-image.png', website: 'http://authorwebsite.com', - facebook: 'https://www.facebook.com/testuser', - twitter: 'https://twitter.com/testuser' + facebook: 'testuser', + twitter: '@testuser' }, authorBk = _.cloneDeep(author); helpers.ghost_head.call( @@ -393,8 +393,8 @@ describe('{{ghost_head}} helper', function () { image: '/content/images/test-author-image.png', website: 'http://authorwebsite.com', bio: 'Author bio', - facebook: 'https://www.facebook.com/testuser', - twitter: 'https://twitter.com/testuser' + facebook: 'testuser', + twitter: '@testuser' } }, postBk = _.cloneDeep(post); @@ -470,8 +470,8 @@ describe('{{ghost_head}} helper', function () { slug: 'Author', image: '/content/images/test-author-image.png', website: 'http://authorwebsite.com', - facebook: 'https://www.facebook.com/testuser', - twitter: 'https://twitter.com/testuser' + facebook: 'testuser', + twitter: '@testuser' } }; @@ -546,8 +546,8 @@ describe('{{ghost_head}} helper', function () { slug: 'Author', image: '/content/images/test-author-image.png', website: 'http://authorwebsite.com', - facebook: 'https://www.facebook.com/testuser', - twitter: 'https://twitter.com/testuser' + facebook: 'testuser', + twitter: '@testuser' } }; @@ -618,8 +618,8 @@ describe('{{ghost_head}} helper', function () { slug: 'Author', image: null, website: 'http://authorwebsite.com', - facebook: 'https://www.facebook.com/testuser', - twitter: 'https://twitter.com/testuser' + facebook: 'testuser', + twitter: '@testuser' } }; @@ -833,8 +833,8 @@ describe('{{ghost_head}} helper', function () { slug: 'Author', image: 'content/images/test-author-image.png', website: 'http://authorwebsite.com', - facebook: 'https://www.facebook.com/testuser', - twitter: 'https://twitter.com/testuser' + facebook: 'testuser', + twitter: '@testuser' } }; From 6dbf610c8f698191600708a8ccbe8c8692eb262d Mon Sep 17 00:00:00 2001 From: Aileen Nowak Date: Mon, 16 May 2016 20:16:40 +0200 Subject: [PATCH 3/4] Fixes error in validation closes #6826 - refactors the validation of facebook and twitter input field in `general.js` and `user.js` controller - Example validations for facebook: - `facebook.com/username` will be corrected to the full URL - `user` will show error `Your Page name is not a valid Facebook Page name' for `general.js` and `Your Username is not a valid Facebook Username` for `user.js` as the username in facebook has to be at least 5 characters long - `twitter.com/username` will be autocorrected to the valid facebook URL incl. the `username` - Example validations for twitter: - `twitter.com/user_` will be corrected to the full URL - `user:99` will show error `Your Username is not a valid Twitter Username` - `facebook.com/username` will be autocorrected to the valid twitter URL incl. the `username` - updates both acceptance tests - adds further validation for facebook pages in general settings and user. Submitting a url which incl. `/page/` or `/pages/` will now accept any username followed incl. further `/`. - adds a custom transform `facebook-url-user` which will extract the username (if it's a facebook page, incl. `pages/`) to store only this in the backend - uses the `twitter-url-user` transform now also for user --- .../app/controllers/settings/general.js | 112 ++++++++++------- core/client/app/controllers/team/user.js | 116 ++++++++++-------- core/client/app/models/setting.js | 2 +- core/client/app/models/user.js | 4 +- .../app/transforms/facebook-url-user.js | 21 ++++ .../tests/acceptance/settings/general-test.js | 58 ++++++++- core/client/tests/acceptance/team-test.js | 42 ++++++- .../unit/transforms/facebook-url-user-test.js | 32 +++++ core/server/data/schema/schema.js | 4 +- 9 files changed, 284 insertions(+), 107 deletions(-) create mode 100644 core/client/app/transforms/facebook-url-user.js create mode 100644 core/client/tests/unit/transforms/facebook-url-user-test.js diff --git a/core/client/app/controllers/settings/general.js b/core/client/app/controllers/settings/general.js index 8599aa9a4e9..66a35107402 100644 --- a/core/client/app/controllers/settings/general.js +++ b/core/client/app/controllers/settings/general.js @@ -134,39 +134,51 @@ export default Controller.extend(SettingsSaveMixin, { // If new url didn't change, exit if (newUrl === oldUrl) { + this.get('model.errors').remove('facebook'); return; } - if (!newUrl.match(/(^https:\/\/www\.facebook\.com\/)(\S+)/g)) { - if (newUrl.match(/(?:facebook\.com\/)(\S+)/) || (!validator.isURL(newUrl) && newUrl.match(/([a-zA-Z0-9\.]+)/))) { - let [ , username] = newUrl.match(/(?:facebook\.com\/)(\S+)/) || newUrl.match(/([a-zA-Z0-9\.]+)/); - newUrl = `https://www.facebook.com/${username}`; + if (newUrl.match(/(?:facebook\.com\/)(\S+)/) || newUrl.match(/([a-z\d\.]+)/i)) { + let username = []; - this.set('model.facebook', newUrl); + if (newUrl.match(/(?:facebook\.com\/)(\S+)/)) { + [ , username ] = newUrl.match(/(?:facebook\.com\/)(\S+)/); + } else { + [ , username ] = newUrl.match(/(?:https\:\/\/|http\:\/\/)?(?:www\.)?(?:\w+\.\w+\/+)?(\S+)/mi); + } - this.get('model.errors').remove('facebook'); - this.get('model.hasValidated').pushObject('facebook'); + // check if we have a /page/username or without + if (username.match(/^(?:\/)?(pages?\/\S+)/mi)) { + // we got a page url, now save the username without the / in the beginning + + [ , username ] = username.match(/^(?:\/)?(pages?\/\S+)/mi); + } else if (username.match(/^(http|www)|(\/)/) || !username.match(/^([a-z\d\.]{5,50})$/mi)) { + errMessage = !username.match(/^([a-z\d\.]{5,50})$/mi) ? 'Your Page name is not a valid Facebook Page name' : 'The URL must be in a format like https://www.facebook.com/yourPage'; - // User input is validated - return this.save().then(() => { - this.set('model.facebook', ''); - run.schedule('afterRender', this, function () { - this.set('model.facebook', newUrl); - }); - }); - } else if (validator.isURL(newUrl)) { - errMessage = 'The URL must be in a format like ' + - 'https://www.facebook.com/yourPage'; - this.get('model.errors').add('facebook', errMessage); - this.get('model.hasValidated').pushObject('facebook'); - return; - } else { - errMessage = 'The URL must be in a format like ' + - 'https://www.facebook.com/yourPage'; this.get('model.errors').add('facebook', errMessage); this.get('model.hasValidated').pushObject('facebook'); return; } + + newUrl = `https://www.facebook.com/${username}`; + this.set('model.facebook', newUrl); + + this.get('model.errors').remove('facebook'); + this.get('model.hasValidated').pushObject('facebook'); + + // User input is validated + return this.save().then(() => { + this.set('model.facebook', ''); + run.schedule('afterRender', this, function () { + this.set('model.facebook', newUrl); + }); + }); + } else { + errMessage = 'The URL must be in a format like ' + + 'https://www.facebook.com/yourPage'; + this.get('model.errors').add('facebook', errMessage); + this.get('model.hasValidated').pushObject('facebook'); + return; } }, @@ -184,39 +196,47 @@ export default Controller.extend(SettingsSaveMixin, { // If new url didn't change, exit if (newUrl === oldUrl) { + this.get('model.errors').remove('twitter'); return; } - if (!newUrl.match(/(^https:\/\/twitter\.com\/)(\S+)/g)) { - if (newUrl.match(/(?:twitter\.com\/)(\S+)/) || (!validator.isURL(newUrl) && newUrl.match(/([a-zA-Z0-9\.]+)/))) { - let [ , username] = newUrl.match(/(?:twitter\.com\/)(\S+)/) || newUrl.match(/([a-zA-Z0-9\.]+)/); - newUrl = `https://twitter.com/${username}`; + if (newUrl.match(/(?:twitter\.com\/)(\S+)/) || newUrl.match(/([a-z\d\.]+)/i)) { + let username = []; - this.set('model.twitter', newUrl); + if (newUrl.match(/(?:twitter\.com\/)(\S+)/)) { + [ , username] = newUrl.match(/(?:twitter\.com\/)(\S+)/); + } else { + [username] = newUrl.match(/([^/]+)\/?$/mi); + } - this.get('model.errors').remove('twitter'); - this.get('model.hasValidated').pushObject('twitter'); + // check if username starts with http or www and show error if so + if (username.match(/^(http|www)|(\/)/) || !username.match(/^[a-z\d\.\_]{1,15}$/mi)) { + errMessage = !username.match(/^[a-z\d\.\_]{1,15}$/mi) ? 'Your Username is not a valid Twitter Username' : 'The URL must be in a format like https://twitter.com/yourUsername'; - // User input is validated - return this.save().then(() => { - this.set('model.twitter', ''); - run.schedule('afterRender', this, function () { - this.set('model.twitter', newUrl); - }); - }); - } else if (validator.isURL(newUrl)) { - errMessage = 'The URL must be in a format like ' + - 'https://twitter.com/yourUsername'; - this.get('model.errors').add('twitter', errMessage); - this.get('model.hasValidated').pushObject('twitter'); - return; - } else { - errMessage = 'The URL must be in a format like ' + - 'https://twitter.com/yourUsername'; this.get('model.errors').add('twitter', errMessage); this.get('model.hasValidated').pushObject('twitter'); return; } + + newUrl = `https://twitter.com/${username}`; + this.set('model.twitter', newUrl); + + this.get('model.errors').remove('twitter'); + this.get('model.hasValidated').pushObject('twitter'); + + // User input is validated + return this.save().then(() => { + this.set('model.twitter', ''); + run.schedule('afterRender', this, function () { + this.set('model.twitter', newUrl); + }); + }); + } else { + errMessage = 'The URL must be in a format like ' + + 'https://twitter.com/yourUsername'; + this.get('model.errors').add('twitter', errMessage); + this.get('model.hasValidated').pushObject('twitter'); + return; } } } diff --git a/core/client/app/controllers/team/user.js b/core/client/app/controllers/team/user.js index 6672c3cf0b9..dfa46fe59bc 100644 --- a/core/client/app/controllers/team/user.js +++ b/core/client/app/controllers/team/user.js @@ -258,41 +258,53 @@ export default Controller.extend({ // If new url didn't change, exit if (newUrl === oldUrl) { + this.get('user.errors').remove('facebook'); return; } // TODO: put the validation here into a validator - if (!newUrl.match(/(^https:\/\/www\.facebook\.com\/)(\S+)/g)) { - if (newUrl.match(/(?:facebook\.com\/)(\S+)/) || (!validator.isURL(newUrl) && newUrl.match(/([a-zA-Z0-9\.]+)/))) { - let [ , username] = newUrl.match(/(?:facebook\.com\/)(\S+)/) || newUrl.match(/([a-zA-Z0-9\.]+)/); - newUrl = `https://www.facebook.com/${username}`; + if (newUrl.match(/(?:facebook\.com\/)(\S+)/) || newUrl.match(/([a-z\d\.]+)/i)) { + let username = []; - this.set('user.facebook', newUrl); + if (newUrl.match(/(?:facebook\.com\/)(\S+)/)) { + [ , username ] = newUrl.match(/(?:facebook\.com\/)(\S+)/); + } else { + [ , username ] = newUrl.match(/(?:https\:\/\/|http\:\/\/)?(?:www\.)?(?:\w+\.\w+\/+)?(\S+)/mi); + } - this.get('user.errors').remove('facebook'); - this.get('user.hasValidated').pushObject('facebook'); + // check if we have a /page/username or without + if (username.match(/^(?:\/)?(pages?\/\S+)/mi)) { + // we got a page url, now save the username without the / in the beginning + + [ , username ] = username.match(/^(?:\/)?(pages?\/\S+)/mi); + } else if (username.match(/^(http|www)|(\/)/) || !username.match(/^([a-z\d\.]{5,50})$/mi)) { + errMessage = !username.match(/^([a-z\d\.]{5,50})$/mi) ? 'Your Username is not a valid Facebook Username' : 'The URL must be in a format like https://www.facebook.com/yourUsername'; - // User input is validated - invoke(this, 'save').then(() => { - // necessary to update the value in the input field - this.set('user.facebook', ''); - run.schedule('afterRender', this, function () { - this.set('user.facebook', newUrl); - }); - }); - } else if (validator.isURL(newUrl)) { - errMessage = 'The URL must be in a format like ' + - 'https://www.facebook.com/yourUsername'; - this.get('user.errors').add('facebook', errMessage); - this.get('user.hasValidated').pushObject('facebook'); - return; - } else { - errMessage = 'The URL must be in a format like ' + - 'https://www.facebook.com/yourUsername'; this.get('user.errors').add('facebook', errMessage); this.get('user.hasValidated').pushObject('facebook'); return; } + + newUrl = `https://www.facebook.com/${username}`; + this.set('user.facebook', newUrl); + + this.get('user.errors').remove('facebook'); + this.get('user.hasValidated').pushObject('facebook'); + + // User input is validated + invoke(this, 'save').then(() => { + // necessary to update the value in the input field + this.set('user.facebook', ''); + run.schedule('afterRender', this, function () { + this.set('user.facebook', newUrl); + }); + }); + } else { + errMessage = 'The URL must be in a format like ' + + 'https://www.facebook.com/yourUsername'; + this.get('user.errors').add('facebook', errMessage); + this.get('user.hasValidated').pushObject('facebook'); + return; } }, @@ -310,41 +322,49 @@ export default Controller.extend({ // If new url didn't change, exit if (newUrl === oldUrl) { + this.get('user.errors').remove('twitter'); return; } // TODO: put the validation here into a validator - if (!newUrl.match(/(^https:\/\/twitter\.com\/)(\S+)/g)) { - if (newUrl.match(/(?:twitter\.com\/)(\S+)/) || (!validator.isURL(newUrl) && newUrl.match(/([a-zA-Z0-9\.]+)/))) { - let [ , username] = newUrl.match(/(?:twitter\.com\/)(\S+)/) || newUrl.match(/([a-zA-Z0-9\.]+)/); - newUrl = `https://twitter.com/${username}`; + if (newUrl.match(/(?:twitter\.com\/)(\S+)/) || newUrl.match(/([a-z\d\.]+)/i)) { + let username = []; - this.set('user.twitter', newUrl); + if (newUrl.match(/(?:twitter\.com\/)(\S+)/)) { + [ , username] = newUrl.match(/(?:twitter\.com\/)(\S+)/); + } else { + [username] = newUrl.match(/([^/]+)\/?$/mi); + } - this.get('user.errors').remove('twitter'); - this.get('user.hasValidated').pushObject('twitter'); + // check if username starts with http or www and show error if so + if (username.match(/^(http|www)|(\/)/) || !username.match(/^[a-z\d\.\_]{1,15}$/mi)) { + errMessage = !username.match(/^[a-z\d\.\_]{1,15}$/mi) ? 'Your Username is not a valid Twitter Username' : 'The URL must be in a format like https://twitter.com/yourUsername'; - // User input is validated - invoke(this, 'save').then(() => { - // necessary to update the value in the input field - this.set('user.twitter', ''); - run.schedule('afterRender', this, function () { - this.set('user.twitter', newUrl); - }); - }); - } else if (validator.isURL(newUrl)) { - errMessage = 'The URL must be in a format like ' + - 'https://twitter.com/yourUsername'; - this.get('user.errors').add('twitter', errMessage); - this.get('user.hasValidated').pushObject('twitter'); - return; - } else { - errMessage = 'The URL must be in a format like ' + - 'https://twitter.com/yourUsername'; this.get('user.errors').add('twitter', errMessage); this.get('user.hasValidated').pushObject('twitter'); return; } + + newUrl = `https://twitter.com/${username}`; + this.set('user.twitter', newUrl); + + this.get('user.errors').remove('twitter'); + this.get('user.hasValidated').pushObject('twitter'); + + // User input is validated + invoke(this, 'save').then(() => { + // necessary to update the value in the input field + this.set('user.twitter', ''); + run.schedule('afterRender', this, function () { + this.set('user.twitter', newUrl); + }); + }); + } else { + errMessage = 'The URL must be in a format like ' + + 'https://twitter.com/yourUsername'; + this.get('user.errors').add('twitter', errMessage); + this.get('user.hasValidated').pushObject('twitter'); + return; } }, diff --git a/core/client/app/models/setting.js b/core/client/app/models/setting.js index c8b25efb74f..22a18849449 100644 --- a/core/client/app/models/setting.js +++ b/core/client/app/models/setting.js @@ -18,7 +18,7 @@ export default Model.extend(ValidationEngine, { availableThemes: attr(), ghost_head: attr('string'), ghost_foot: attr('string'), - facebook: attr('string'), + facebook: attr('facebook-url-user'), twitter: attr('twitter-url-user'), labs: attr('string'), navigation: attr('navigation-settings'), diff --git a/core/client/app/models/user.js b/core/client/app/models/user.js index 225bbb343e3..e3d3dab14f6 100644 --- a/core/client/app/models/user.js +++ b/core/client/app/models/user.js @@ -38,8 +38,8 @@ export default Model.extend(ValidationEngine, { async: false }), count: attr('raw'), - facebook: attr('string'), - twitter: attr('string'), + facebook: attr('facebook-url-user'), + twitter: attr('twitter-url-user'), ghostPaths: service(), ajax: service(), diff --git a/core/client/app/transforms/facebook-url-user.js b/core/client/app/transforms/facebook-url-user.js new file mode 100644 index 00000000000..bb2f5e7825b --- /dev/null +++ b/core/client/app/transforms/facebook-url-user.js @@ -0,0 +1,21 @@ +import Transform from 'ember-data/transform'; + +export default Transform.extend({ + deserialize(serialized) { + if (serialized) { + let [ , user ] = serialized.match(/(\S+)/); + + return `https://www.facebook.com/${user}`; + } + return serialized; + }, + + serialize(deserialized) { + if (deserialized) { + let [ , user] = deserialized.match(/(?:https:\/\/)(?:www\.)(?:facebook\.com)\/(?:#!\/)?(\w+\/?\S+)/mi); + + return user; + } + return deserialized; + } +}); diff --git a/core/client/tests/acceptance/settings/general-test.js b/core/client/tests/acceptance/settings/general-test.js index 05d2a5d5025..27800cccc63 100644 --- a/core/client/tests/acceptance/settings/general-test.js +++ b/core/client/tests/acceptance/settings/general-test.js @@ -172,6 +172,15 @@ describe('Acceptance: Settings - General', function () { .to.equal(''); }); + fillIn('#settings-general input[name="general[facebook]"]', 'facebook.com/pages/some-facebook-page/857469375913?ref=ts'); + triggerEvent('#settings-general input[name="general[facebook]"]', 'blur'); + + andThen(() => { + expect(find('#settings-general input[name="general[facebook]"]').val()).to.be.equal('https://www.facebook.com/pages/some-facebook-page/857469375913?ref=ts'); + expect(find('#settings-general .error .response').text().trim(), 'inline validation response') + .to.equal(''); + }); + fillIn('#settings-general input[name="general[facebook]"]', '*(&*(%%))'); triggerEvent('#settings-general input[name="general[facebook]"]', 'blur'); @@ -184,8 +193,18 @@ describe('Acceptance: Settings - General', function () { triggerEvent('#settings-general input[name="general[facebook]"]', 'blur'); andThen(() => { + expect(find('#settings-general input[name="general[facebook]"]').val()).to.be.equal('https://www.facebook.com/username'); expect(find('#settings-general .error .response').text().trim(), 'inline validation response') - .to.equal('The URL must be in a format like https://www.facebook.com/yourPage'); + .to.equal(''); + }); + + fillIn('#settings-general input[name="general[facebook]"]', 'http://github.com/pages/username'); + triggerEvent('#settings-general input[name="general[facebook]"]', 'blur'); + + andThen(() => { + expect(find('#settings-general input[name="general[facebook]"]').val()).to.be.equal('https://www.facebook.com/pages/username'); + expect(find('#settings-general .error .response').text().trim(), 'inline validation response') + .to.equal(''); }); fillIn('#settings-general input[name="general[facebook]"]', 'testuser'); @@ -197,6 +216,32 @@ describe('Acceptance: Settings - General', function () { .to.equal(''); }); + fillIn('#settings-general input[name="general[facebook]"]', 'ab99'); + triggerEvent('#settings-general input[name="general[facebook]"]', 'blur'); + + andThen(() => { + expect(find('#settings-general .error .response').text().trim(), 'inline validation response') + .to.equal('Your Page name is not a valid Facebook Page name'); + }); + + fillIn('#settings-general input[name="general[facebook]"]', 'page/ab99'); + triggerEvent('#settings-general input[name="general[facebook]"]', 'blur'); + + andThen(() => { + expect(find('#settings-general input[name="general[facebook]"]').val()).to.be.equal('https://www.facebook.com/page/ab99'); + expect(find('#settings-general .error .response').text().trim(), 'inline validation response') + .to.equal(''); + }); + + fillIn('#settings-general input[name="general[facebook]"]', 'page/*(&*(%%))'); + triggerEvent('#settings-general input[name="general[facebook]"]', 'blur'); + + andThen(() => { + expect(find('#settings-general input[name="general[facebook]"]').val()).to.be.equal('https://www.facebook.com/page/*(&*(%%))'); + expect(find('#settings-general .error .response').text().trim(), 'inline validation response') + .to.equal(''); + }); + // validates a twitter url correctly fillIn('#settings-general input[name="general[twitter]"]', 'twitter.com/username'); triggerEvent('#settings-general input[name="general[twitter]"]', 'blur'); @@ -219,8 +264,17 @@ describe('Acceptance: Settings - General', function () { triggerEvent('#settings-general input[name="general[twitter]"]', 'blur'); andThen(() => { + expect(find('#settings-general input[name="general[twitter]"]').val()).to.be.equal('https://twitter.com/username'); expect(find('#settings-general .error .response').text().trim(), 'inline validation response') - .to.equal('The URL must be in a format like https://twitter.com/yourUsername'); + .to.equal(''); + }); + + fillIn('#settings-general input[name="general[twitter]"]', 'thisusernamehasmorethan15characters'); + triggerEvent('#settings-general input[name="general[twitter]"]', 'blur'); + + andThen(() => { + expect(find('#settings-general .error .response').text().trim(), 'inline validation response') + .to.equal('Your Username is not a valid Twitter Username'); }); fillIn('#settings-general input[name="general[twitter]"]', 'testuser'); diff --git a/core/client/tests/acceptance/team-test.js b/core/client/tests/acceptance/team-test.js index a69f4fb55fd..c0b9eb8a0da 100644 --- a/core/client/tests/acceptance/team-test.js +++ b/core/client/tests/acceptance/team-test.js @@ -313,6 +313,7 @@ describe('Acceptance: Team', function () { expect(find('.user-details-bottom .form-group:nth-of-type(4)').hasClass('error'), 'website input should be in error state').to.be.true; }); + // Testing Facebook input fillIn('#user-facebook', ''); fillIn('#user-facebook', ')(*&%^%)'); triggerEvent('#user-facebook', 'blur'); @@ -322,16 +323,34 @@ describe('Acceptance: Team', function () { }); fillIn('#user-facebook', ''); - fillIn('#user-facebook', 'name'); + fillIn('#user-facebook', 'pages/)(*&%^%)'); triggerEvent('#user-facebook', 'blur'); andThen(() => { - expect(find('#user-facebook').val()).to.be.equal('https://www.facebook.com/name'); + expect(find('#user-facebook').val()).to.be.equal('https://www.facebook.com/pages/)(*&%^%)'); expect(find('.user-details-bottom .form-group:nth-of-type(5)').hasClass('error'), 'facebook input should be in error state').to.be.false; }); fillIn('#user-facebook', ''); - fillIn('#user-facebook', 'http://twitter.com/user'); + fillIn('#user-facebook', 'testing'); + triggerEvent('#user-facebook', 'blur'); + + andThen(() => { + expect(find('#user-facebook').val()).to.be.equal('https://www.facebook.com/testing'); + expect(find('.user-details-bottom .form-group:nth-of-type(5)').hasClass('error'), 'facebook input should be in error state').to.be.false; + }); + + fillIn('#user-facebook', ''); + fillIn('#user-facebook', 'somewebsite.com/pages/some-facebook-page/857469375913?ref=ts'); + triggerEvent('#user-facebook', 'blur'); + + andThen(() => { + expect(find('#user-facebook').val()).to.be.equal('https://www.facebook.com/pages/some-facebook-page/857469375913?ref=ts'); + expect(find('.user-details-bottom .form-group:nth-of-type(5)').hasClass('error'), 'facebook input should be in error state').to.be.false; + }); + + fillIn('#user-facebook', ''); + fillIn('#user-facebook', 'test'); triggerEvent('#user-facebook', 'blur'); andThen(() => { @@ -339,14 +358,24 @@ describe('Acceptance: Team', function () { }); fillIn('#user-facebook', ''); - fillIn('#user-facebook', 'facebook.com/user'); + fillIn('#user-facebook', 'http://twitter.com/testuser'); + triggerEvent('#user-facebook', 'blur'); + + andThen(() => { + expect(find('#user-facebook').val()).to.be.equal('https://www.facebook.com/testuser'); + expect(find('.user-details-bottom .form-group:nth-of-type(5)').hasClass('error'), 'facebook input should be in error state').to.be.false; + }); + + fillIn('#user-facebook', ''); + fillIn('#user-facebook', 'facebook.com/testing'); triggerEvent('#user-facebook', 'blur'); andThen(() => { - expect(find('#user-facebook').val()).to.be.equal('https://www.facebook.com/user'); + expect(find('#user-facebook').val()).to.be.equal('https://www.facebook.com/testing'); expect(find('.user-details-bottom .form-group:nth-of-type(5)').hasClass('error'), 'facebook input should be in error state').to.be.false; }); + // Testing Twitter input fillIn('#user-twitter', ''); fillIn('#user-twitter', ')(*&%^%)'); triggerEvent('#user-twitter', 'blur'); @@ -369,7 +398,8 @@ describe('Acceptance: Team', function () { triggerEvent('#user-twitter', 'blur'); andThen(() => { - expect(find('.user-details-bottom .form-group:nth-of-type(6)').hasClass('error'), 'twitter input should be in error state').to.be.true; + expect(find('#user-twitter').val()).to.be.equal('https://twitter.com/user'); + expect(find('.user-details-bottom .form-group:nth-of-type(6)').hasClass('error'), 'twitter input should be in error state').to.be.false; }); fillIn('#user-twitter', ''); diff --git a/core/client/tests/unit/transforms/facebook-url-user-test.js b/core/client/tests/unit/transforms/facebook-url-user-test.js new file mode 100644 index 00000000000..dfef25bcc9e --- /dev/null +++ b/core/client/tests/unit/transforms/facebook-url-user-test.js @@ -0,0 +1,32 @@ +/* jshint expr:true */ +import { expect } from 'chai'; +import { describeModule, it } from 'ember-mocha'; +import Ember from 'ember'; + +const emberA = Ember.A; + +describeModule( + 'transform:facebook-url-user', + 'Unit: Transform: facebook-url-user', + { + // Specify the other units that are required for this test. + // needs: ['transform:foo'] + }, + function() { + it('deserializes facebook url', function () { + let transform = this.subject(); + let serialized = 'testuser'; + let result = transform.deserialize(serialized); + + expect(result).to.equal('https://www.facebook.com/testuser'); + }); + + it('serializes url to facebook username', function () { + let transform = this.subject(); + let deserialized = 'https://www.facebook.com/testuser'; + let result = transform.serialize(deserialized); + + expect(result).to.equal('testuser'); + }); + } +); diff --git a/core/server/data/schema/schema.js b/core/server/data/schema/schema.js index 4b2b810240d..2fb2ce4e145 100644 --- a/core/server/data/schema/schema.js +++ b/core/server/data/schema/schema.js @@ -35,8 +35,8 @@ module.exports = { bio: {type: 'string', maxlength: 200, nullable: true}, website: {type: 'text', maxlength: 2000, nullable: true, validations: {isEmptyOrURL: true}}, location: {type: 'text', maxlength: 65535, nullable: true}, - facebook: {type: 'text', maxlength: 2000, nullable: true, validations: {isEmptyOrURL: true}}, - twitter: {type: 'text', maxlength: 2000, nullable: true, validations: {isEmptyOrURL: true}}, + facebook: {type: 'text', maxlength: 2000, nullable: true}, + twitter: {type: 'text', maxlength: 2000, nullable: true}, accessibility: {type: 'text', maxlength: 65535, nullable: true}, status: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'active'}, language: {type: 'string', maxlength: 6, nullable: false, defaultTo: 'en_US'}, From 2640f774df769b29066e9ba1309e4d32e280f4a3 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Tue, 17 May 2016 19:14:14 +0100 Subject: [PATCH 4/4] Fix unwanted clearing of social inputs on blur with no edits no issue - updates the logic to expect `null` scratch values as the scratch values won't be set until the input value has changed - adds tests for initial value display and regression tests for the cleared input bug --- .../app/controllers/settings/general.js | 16 ++++++-- core/client/app/controllers/team/user.js | 14 ++++++- core/client/app/mirage/fixtures/settings.js | 4 +- .../tests/acceptance/settings/general-test.js | 34 +++++++++++++++ core/client/tests/acceptance/team-test.js | 41 ++++++++++++++++++- 5 files changed, 101 insertions(+), 8 deletions(-) diff --git a/core/client/app/controllers/settings/general.js b/core/client/app/controllers/settings/general.js index 66a35107402..92938e2ea7a 100644 --- a/core/client/app/controllers/settings/general.js +++ b/core/client/app/controllers/settings/general.js @@ -125,13 +125,18 @@ export default Controller.extend(SettingsSaveMixin, { let oldUrl = this.get('model.facebook'); let errMessage = ''; - if (!newUrl) { + if (newUrl === '') { // Clear out the Facebook url this.set('model.facebook', ''); this.get('model.errors').remove('facebook'); return; } + // _scratchFacebook will be null unless the user has input something + if (!newUrl) { + newUrl = oldUrl; + } + // If new url didn't change, exit if (newUrl === oldUrl) { this.get('model.errors').remove('facebook'); @@ -187,13 +192,18 @@ export default Controller.extend(SettingsSaveMixin, { let oldUrl = this.get('model.twitter'); let errMessage = ''; - if (!newUrl) { - // Clear out the Facebook url + if (newUrl === '') { + // Clear out the Twitter url this.set('model.twitter', ''); this.get('model.errors').remove('twitter'); return; } + // _scratchTwitter will be null unless the user has input something + if (!newUrl) { + newUrl = oldUrl; + } + // If new url didn't change, exit if (newUrl === oldUrl) { this.get('model.errors').remove('twitter'); diff --git a/core/client/app/controllers/team/user.js b/core/client/app/controllers/team/user.js index dfa46fe59bc..580a3ffc698 100644 --- a/core/client/app/controllers/team/user.js +++ b/core/client/app/controllers/team/user.js @@ -249,13 +249,18 @@ export default Controller.extend({ let oldUrl = this.get('user.facebook'); let errMessage = ''; - if (!newUrl) { + if (newUrl === '') { // Clear out the Facebook url this.set('user.facebook', ''); this.get('user.errors').remove('facebook'); return; } + // _scratchFacebook will be null unless the user has input something + if (!newUrl) { + newUrl = oldUrl; + } + // If new url didn't change, exit if (newUrl === oldUrl) { this.get('user.errors').remove('facebook'); @@ -313,13 +318,18 @@ export default Controller.extend({ let oldUrl = this.get('user.twitter'); let errMessage = ''; - if (!newUrl) { + if (newUrl === '') { // Clear out the Twitter url this.set('user.twitter', ''); this.get('user.errors').remove('twitter'); return; } + // _scratchTwitter will be null unless the user has input something + if (!newUrl) { + newUrl = oldUrl; + } + // If new url didn't change, exit if (newUrl === oldUrl) { this.get('user.errors').remove('twitter'); diff --git a/core/client/app/mirage/fixtures/settings.js b/core/client/app/mirage/fixtures/settings.js index a560e5fa7e3..472c9a00e30 100644 --- a/core/client/app/mirage/fixtures/settings.js +++ b/core/client/app/mirage/fixtures/settings.js @@ -188,7 +188,7 @@ export default [ updated_at: '2016-05-08T15:20:25.953Z', updated_by: 1, uuid: 'd4387e5c-3230-46dd-a89b-0d8a40365c35', - value: '' + value: 'test' }, { created_at: '2016-05-05T15:40:12.134Z', @@ -199,7 +199,7 @@ export default [ updated_at: '2016-05-08T15:20:25.954Z', updated_by: 1, uuid: '5130441f-e4c7-4750-9692-a22d841ab049', - value: '' + value: '@test' }, { key: 'availableThemes', diff --git a/core/client/tests/acceptance/settings/general-test.js b/core/client/tests/acceptance/settings/general-test.js index 27800cccc63..c4f10f41db5 100644 --- a/core/client/tests/acceptance/settings/general-test.js +++ b/core/client/tests/acceptance/settings/general-test.js @@ -163,6 +163,23 @@ describe('Acceptance: Settings - General', function () { }); // validates a facebook url correctly + + andThen(() => { + // loads fixtures and performs transform + expect(find('input[name="general[facebook]"]').val(), 'initial facebook value') + .to.equal('https://www.facebook.com/test'); + }); + + triggerEvent('#settings-general input[name="general[facebook]"]', 'focus'); + triggerEvent('#settings-general input[name="general[facebook]"]', 'blur'); + + andThen(() => { + // regression test: we still have a value after the input is + // focused and then blurred without any changes + expect(find('input[name="general[facebook]"]').val(), 'facebook value after blur with no change') + .to.equal('https://www.facebook.com/test'); + }); + fillIn('#settings-general input[name="general[facebook]"]', 'facebook.com/username'); triggerEvent('#settings-general input[name="general[facebook]"]', 'blur'); @@ -243,6 +260,23 @@ describe('Acceptance: Settings - General', function () { }); // validates a twitter url correctly + + andThen(() => { + // loads fixtures and performs transform + expect(find('input[name="general[twitter]"]').val(), 'initial twitter value') + .to.equal('https://twitter.com/test'); + }); + + triggerEvent('#settings-general input[name="general[twitter]"]', 'focus'); + triggerEvent('#settings-general input[name="general[twitter]"]', 'blur'); + + andThen(() => { + // regression test: we still have a value after the input is + // focused and then blurred without any changes + expect(find('input[name="general[twitter]"]').val(), 'twitter value after blur with no change') + .to.equal('https://twitter.com/test'); + }); + fillIn('#settings-general input[name="general[twitter]"]', 'twitter.com/username'); triggerEvent('#settings-general input[name="general[twitter]"]', 'blur'); diff --git a/core/client/tests/acceptance/team-test.js b/core/client/tests/acceptance/team-test.js index c0b9eb8a0da..62910d1c2d2 100644 --- a/core/client/tests/acceptance/team-test.js +++ b/core/client/tests/acceptance/team-test.js @@ -239,7 +239,12 @@ describe('Acceptance: Team', function () { let user; beforeEach(function () { - server.create('user', {slug: 'test-1', name: 'Test User'}); + server.create('user', { + slug: 'test-1', + name: 'Test User', + facebook: 'test', + twitter: '@test' + }); server.loadFixtures(); }); @@ -314,6 +319,23 @@ describe('Acceptance: Team', function () { }); // Testing Facebook input + + andThen(() => { + // displays initial value + expect(find('#user-facebook').val(), 'initial facebook value') + .to.equal('https://www.facebook.com/test'); + }); + + triggerEvent('#user-facebook', 'focus'); + triggerEvent('#user-facebook', 'blur'); + + andThen(() => { + // regression test: we still have a value after the input is + // focused and then blurred without any changes + expect(find('#user-facebook').val(), 'facebook value after blur with no change') + .to.equal('https://www.facebook.com/test'); + }); + fillIn('#user-facebook', ''); fillIn('#user-facebook', ')(*&%^%)'); triggerEvent('#user-facebook', 'blur'); @@ -376,6 +398,23 @@ describe('Acceptance: Team', function () { }); // Testing Twitter input + + andThen(() => { + // loads fixtures and performs transform + expect(find('#user-twitter').val(), 'initial twitter value') + .to.equal('https://twitter.com/test'); + }); + + triggerEvent('#user-twitter', 'focus'); + triggerEvent('#user-twitter', 'blur'); + + andThen(() => { + // regression test: we still have a value after the input is + // focused and then blurred without any changes + expect(find('#user-twitter').val(), 'twitter value after blur with no change') + .to.equal('https://twitter.com/test'); + }); + fillIn('#user-twitter', ''); fillIn('#user-twitter', ')(*&%^%)'); triggerEvent('#user-twitter', 'blur');