Signup form: prefill username if Discourse Hub has a match for the email address. Also, fix some bad specs in username_checker_service_spec that were passing...

This commit is contained in:
Neil Lalonde 2013-11-19 14:15:05 -05:00
parent 309904ef8f
commit 981d8f6aea
8 changed files with 148 additions and 64 deletions

View file

@ -15,6 +15,7 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
accountChallenge: 0, accountChallenge: 0,
formSubmitted: false, formSubmitted: false,
rejectedEmails: Em.A([]), rejectedEmails: Em.A([]),
prefilledUsername: null,
submitDisabled: function() { submitDisabled: function() {
if (this.get('formSubmitted')) return true; if (this.get('formSubmitted')) return true;
@ -95,6 +96,28 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
}); });
}.property('accountEmail', 'rejectedEmails.@each'), }.property('accountEmail', 'rejectedEmails.@each'),
prefillUsername: function() {
if (this.get('prefilledUsername')) {
if (this.get('accountUsername') === this.get('prefilledUsername')) {
this.set('accountUsername', '');
}
this.set('prefilledUsername', null);
}
if (this.get('emailValidation.ok') && this.blank('accountUsername')) {
this.fetchExistingUsername();
}
}.observes('emailValidation', 'accountEmail'),
fetchExistingUsername: Discourse.debounce(function() {
var self = this;
Discourse.User.checkUsername(null, this.get('accountEmail')).then(function(result) {
if (result.suggestion && self.blank('accountUsername')) {
self.set('accountUsername', result.suggestion);
self.set('prefilledUsername', result.suggestion);
}
});
}, 500),
usernameMatch: function() { usernameMatch: function() {
if (this.usernameNeedsToBeValidatedWithEmail()) { if (this.usernameNeedsToBeValidatedWithEmail()) {
if (this.get('emailValidation.failed')) { if (this.get('emailValidation.failed')) {
@ -119,6 +142,13 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF
basicUsernameValidation: function() { basicUsernameValidation: function() {
this.set('uniqueUsernameValidation', null); this.set('uniqueUsernameValidation', null);
if (this.get('accountUsername') === this.get('prefilledUsername')) {
return Discourse.InputValidation.create({
ok: true,
reason: I18n.t('user.username.prefilled')
});
}
// If blank, fail without a reason // If blank, fail without a reason
if (this.blank('accountUsername')) { if (this.blank('accountUsername')) {
return Discourse.InputValidation.create({ return Discourse.InputValidation.create({

View file

@ -97,7 +97,10 @@ class UsersController < ApplicationController
# Used for checking availability of a username and will return suggestions # Used for checking availability of a username and will return suggestions
# if the username is not available. # if the username is not available.
def check_username def check_username
params.require(:username) if !params[:username].present?
params.require(:username) if !params[:email].present?
return render(json: success_json) unless SiteSetting.call_discourse_hub?
end
username = params[:username] username = params[:username]
target_user = user_from_params_or_current_user target_user = user_from_params_or_current_user
@ -107,7 +110,7 @@ class UsersController < ApplicationController
checker = UsernameCheckerService.new checker = UsernameCheckerService.new
email = params[:email] || target_user.try(:email) email = params[:email] || target_user.try(:email)
render(json: checker.check_username(username, email)) render json: checker.check_username(username, email)
rescue RestClient::Forbidden rescue RestClient::Forbidden
render json: {errors: [I18n.t("discourse_hub.access_token_problem")]} render json: {errors: [I18n.t("discourse_hub.access_token_problem")]}
end end

View file

@ -1,6 +1,7 @@
class UsernameCheckerService class UsernameCheckerService
def check_username(username, email) def check_username(username, email)
if username && username.length > 0
validator = UsernameValidator.new(username) validator = UsernameValidator.new(username)
if !validator.valid_format? if !validator.valid_format?
{errors: validator.errors} {errors: validator.errors}
@ -9,7 +10,9 @@ class UsernameCheckerService
else else
check_username_with_hub_server(username, email) check_username_with_hub_server(username, email)
end end
elsif email and SiteSetting.call_discourse_hub?
{suggestion: DiscourseHub.nickname_for_email(email)}
end
end end
# Contact the Discourse Hub server # Contact the Discourse Hub server

View file

@ -280,6 +280,7 @@ en:
too_long: "Your username is too long." too_long: "Your username is too long."
checking: "Checking username availability..." checking: "Checking username availability..."
enter_email: 'Username found. Enter matching email.' enter_email: 'Username found. Enter matching email.'
prefilled: "Email matches this registered username."
password_confirmation: password_confirmation:
title: "Password Again" title: "Password Again"

View file

@ -32,6 +32,11 @@ module DiscourseHub
[json['match'], json['available'] || false, json['suggestion']] [json['match'], json['available'] || false, json['suggestion']]
end end
def self.nickname_for_email(email)
json = get('/users/nickname_match', {email: email})
json['suggestion']
end
def self.register_nickname(nickname, email) def self.register_nickname(nickname, email)
json = post('/users', {nickname: nickname, email: email}) json = post('/users', {nickname: nickname, email: email})
if json.has_key?('success') if json.has_key?('success')
@ -100,7 +105,7 @@ module DiscourseHub
if Rails.env == 'production' if Rails.env == 'production'
'http://api.discourse.org/api' 'http://api.discourse.org/api'
else else
'http://local.hub:3000/api' ENV['HUB_BASE_URL'] || 'http://local.hub:3000/api'
end end
end end

View file

@ -545,7 +545,7 @@ describe UsersController do
DiscourseHub.stubs(:nickname_available?).returns([true, nil]) DiscourseHub.stubs(:nickname_available?).returns([true, nil])
end end
it 'raises an error without a username parameter' do it 'raises an error without any parameters' do
lambda { xhr :get, :check_username }.should raise_error(ActionController::ParameterMissing) lambda { xhr :get, :check_username }.should raise_error(ActionController::ParameterMissing)
end end
@ -580,29 +580,19 @@ describe UsersController do
DiscourseHub.expects(:nickname_match?).never DiscourseHub.expects(:nickname_match?).never
end end
context 'available everywhere' do it 'returns nothing when given an email param but no username' do
xhr :get, :check_username, email: 'dood@example.com'
response.should be_success
end
context 'username is available' do
before do before do
xhr :get, :check_username, username: 'BruceWayne' xhr :get, :check_username, username: 'BruceWayne'
end end
include_examples 'when username is available everywhere' include_examples 'when username is available everywhere'
end end
context 'available locally but not globally' do context 'username is unavailable' do
before do
xhr :get, :check_username, username: 'BruceWayne'
end
include_examples 'when username is available everywhere'
end
context 'unavailable locally but available globally' do
let!(:user) { Fabricate(:user) }
before do
xhr :get, :check_username, username: user.username
end
include_examples 'when username is unavailable locally'
end
context 'unavailable everywhere' do
let!(:user) { Fabricate(:user) } let!(:user) { Fabricate(:user) }
before do before do
xhr :get, :check_username, username: user.username xhr :get, :check_username, username: user.username
@ -641,7 +631,7 @@ describe UsersController do
end end
include_examples 'checking an invalid username' include_examples 'checking an invalid username'
it 'should return the "too short" message' do it 'should return the "too long" message' do
::JSON.parse(response.body)['errors'].should include(I18n.t(:'user.username.long', max: User.username_length.end)) ::JSON.parse(response.body)['errors'].should include(I18n.t(:'user.username.long', max: User.username_length.end))
end end
end end
@ -679,12 +669,20 @@ describe UsersController do
include_examples 'check_username when nickname is available everywhere' include_examples 'check_username when nickname is available everywhere'
end end
context 'and email is given' do context 'both username and email is given' do
before do before do
xhr :get, :check_username, username: 'BruceWayne', email: 'brucie@gmail.com' xhr :get, :check_username, username: 'BruceWayne', email: 'brucie@gmail.com'
end end
include_examples 'check_username when nickname is available everywhere' include_examples 'check_username when nickname is available everywhere'
end end
context 'only email is given' do
it "should check for a matching username" do
UsernameCheckerService.any_instance.expects(:check_username).with(nil, 'brucie@gmail.com').returns({json: 'blah'})
xhr :get, :check_username, email: 'brucie@gmail.com'
response.should be_success
end
end
end end
shared_examples 'when email is needed to check nickname match' do shared_examples 'when email is needed to check nickname match' do

View file

@ -11,10 +11,6 @@ describe UsernameCheckerService do
end end
context 'Username invalid' do context 'Username invalid' do
it 'rejects blank usernames' do
result = @service.check_username('', @nil_email)
expect(result).to have_key(:errors)
end
it 'rejects too short usernames' do it 'rejects too short usernames' do
result = @service.check_username('a', @nil_email) result = @service.check_username('a', @nil_email)
expect(result).to have_key(:errors) expect(result).to have_key(:errors)
@ -40,29 +36,27 @@ describe UsernameCheckerService do
SiteSetting.stubs(:call_discourse_hub?).returns(true) SiteSetting.stubs(:call_discourse_hub?).returns(true)
end end
context 'and email is given' do context 'username and email is given' do
it 'username is available locally but not globally' do it 'username is available locally but not globally' do
DiscourseHub.stubs(:nickname_available?).returns([false, 'suggestion']) DiscourseHub.expects(:nickname_available?).never
DiscourseHub.stubs(:nickname_match?).returns([true, false, nil]) DiscourseHub.expects(:nickname_match?).returns([false, false, 'porkchop'])
result = @service.check_username('vincent', @email)
expected = { available: false, global_match: false, suggestion: 'porkchop' }
expect(result).to eq(expected)
end
it 'username is available both locally and globally' do
DiscourseHub.expects(:nickname_available?).never
DiscourseHub.stubs(:nickname_match?).returns([true, true, nil])
result = @service.check_username('vincent', @email) result = @service.check_username('vincent', @email)
expected = { available: true, global_match: true } expected = { available: true, global_match: true }
expect(result).to eq(expected) expect(result).to eq(expected)
end end
end
it 'username is available both locally and globally' do
DiscourseHub.stubs(:nickname_available?).returns([true, nil])
DiscourseHub.stubs(:nickname_match?).returns([false, true, nil])
result = @service.check_username('vincent', @email)
expected = { available: true, global_match: false }
expect(result).to eq(expected)
end
it 'username is available locally but not globally' do it 'username is available locally but not globally' do
DiscourseHub.stubs(:nickname_match?).returns([false, true, nil]) DiscourseHub.stubs(:nickname_match?).returns([false, false, 'suggestion'])
result = @service.check_username('vincent', @email) result = @service.check_username('vincent', @email)
expected = { available: true, global_match: false } expected = { available: false, global_match: false, suggestion: 'suggestion' }
expect(result).to eq(expected) expect(result).to eq(expected)
end end
@ -84,6 +78,36 @@ describe UsernameCheckerService do
end end
end end
shared_examples "only email is given" do
it "should call the correct api" do
DiscourseHub.expects(:nickname_available?).never
DiscourseHub.expects(:nickname_match?).never
DiscourseHub.stubs(:nickname_for_email).returns(nil)
result
end
it 'no match on email' do
DiscourseHub.stubs(:nickname_for_email).returns(nil)
result.should == {suggestion: nil}
end
it 'match found for email' do
DiscourseHub.stubs(:nickname_for_email).returns('vincent')
result.should == {suggestion: 'vincent'}
end
end
context 'username is nil' do
subject(:result) { @service.check_username(nil, @email) }
include_examples "only email is given"
end
context 'username is empty string' do
subject(:result) { @service.check_username('', @email) }
include_examples "only email is given"
end
end
context 'Discourse Hub disabled' do context 'Discourse Hub disabled' do
it 'username not available locally' do it 'username not available locally' do
User.stubs(:username_available?).returns(false) User.stubs(:username_available?).returns(false)

View file

@ -0,0 +1,20 @@
module("Discourse.CreateAccountController");
test('basicUsernameValidation', function() {
var testInvalidUsername = function(username, expectedReason) {
var controller = testController(Discourse.CreateAccountController, null);
controller.set('accountUsername', username);
equal(controller.get('basicUsernameValidation.failed'), true, 'username should be invalid: ' + username);
equal(controller.get('basicUsernameValidation.reason'), expectedReason, 'username validation reason: ' + username + ', ' + expectedReason);
};
testInvalidUsername('', undefined);
testInvalidUsername('x', I18n.t('user.username.too_short'));
testInvalidUsername('1234567890123456', I18n.t('user.username.too_long'));
var controller = testController(Discourse.CreateAccountController, null);
controller.set('accountUsername', 'porkchops');
controller.set('prefilledUsername', 'porkchops');
equal(controller.get('basicUsernameValidation.ok'), true, 'Prefilled username is valid');
equal(controller.get('basicUsernameValidation.reason'), I18n.t('user.username.prefilled'), 'Prefilled username is valid');
});