diff --git a/app/assets/javascripts/discourse/controllers/create_account_controller.js b/app/assets/javascripts/discourse/controllers/create_account_controller.js index 7f48f718fd7..81b1cb52169 100644 --- a/app/assets/javascripts/discourse/controllers/create_account_controller.js +++ b/app/assets/javascripts/discourse/controllers/create_account_controller.js @@ -15,6 +15,7 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF accountChallenge: 0, formSubmitted: false, rejectedEmails: Em.A([]), + prefilledUsername: null, submitDisabled: function() { if (this.get('formSubmitted')) return true; @@ -95,6 +96,28 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF }); }.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() { if (this.usernameNeedsToBeValidatedWithEmail()) { if (this.get('emailValidation.failed')) { @@ -119,6 +142,13 @@ Discourse.CreateAccountController = Discourse.Controller.extend(Discourse.ModalF basicUsernameValidation: function() { 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 (this.blank('accountUsername')) { return Discourse.InputValidation.create({ diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 42beb0386af..26c52b86d2b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -97,7 +97,10 @@ class UsersController < ApplicationController # Used for checking availability of a username and will return suggestions # if the username is not available. 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] target_user = user_from_params_or_current_user @@ -107,7 +110,7 @@ class UsersController < ApplicationController checker = UsernameCheckerService.new email = params[:email] || target_user.try(:email) - render(json: checker.check_username(username, email)) + render json: checker.check_username(username, email) rescue RestClient::Forbidden render json: {errors: [I18n.t("discourse_hub.access_token_problem")]} end diff --git a/app/services/username_checker_service.rb b/app/services/username_checker_service.rb index 24ec615e963..71c6d9a2b79 100644 --- a/app/services/username_checker_service.rb +++ b/app/services/username_checker_service.rb @@ -1,15 +1,18 @@ class UsernameCheckerService def check_username(username, email) - validator = UsernameValidator.new(username) - if !validator.valid_format? - {errors: validator.errors} - elsif !SiteSetting.call_discourse_hub? - check_username_locally(username) - else - check_username_with_hub_server(username, email) + if username && username.length > 0 + validator = UsernameValidator.new(username) + if !validator.valid_format? + {errors: validator.errors} + elsif !SiteSetting.call_discourse_hub? + check_username_locally(username) + else + check_username_with_hub_server(username, email) + end + elsif email and SiteSetting.call_discourse_hub? + {suggestion: DiscourseHub.nickname_for_email(email)} end - end # Contact the Discourse Hub server diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index c5e44031710..0fe64bda2b9 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -280,6 +280,7 @@ en: too_long: "Your username is too long." checking: "Checking username availability..." enter_email: 'Username found. Enter matching email.' + prefilled: "Email matches this registered username." password_confirmation: title: "Password Again" diff --git a/lib/discourse_hub.rb b/lib/discourse_hub.rb index 9e699e184d4..e4e76f1913a 100644 --- a/lib/discourse_hub.rb +++ b/lib/discourse_hub.rb @@ -32,6 +32,11 @@ module DiscourseHub [json['match'], json['available'] || false, json['suggestion']] end + def self.nickname_for_email(email) + json = get('/users/nickname_match', {email: email}) + json['suggestion'] + end + def self.register_nickname(nickname, email) json = post('/users', {nickname: nickname, email: email}) if json.has_key?('success') @@ -100,7 +105,7 @@ module DiscourseHub if Rails.env == 'production' 'http://api.discourse.org/api' else - 'http://local.hub:3000/api' + ENV['HUB_BASE_URL'] || 'http://local.hub:3000/api' end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 41232e25ca1..4c66603b8c3 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -545,7 +545,7 @@ describe UsersController do DiscourseHub.stubs(:nickname_available?).returns([true, nil]) 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) end @@ -580,29 +580,19 @@ describe UsersController do DiscourseHub.expects(:nickname_match?).never 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 xhr :get, :check_username, username: 'BruceWayne' end include_examples 'when username is available everywhere' end - context 'available locally but not globally' 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 + context 'username is unavailable' do let!(:user) { Fabricate(:user) } before do xhr :get, :check_username, username: user.username @@ -641,7 +631,7 @@ describe UsersController do end 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)) end end @@ -679,12 +669,20 @@ describe UsersController do include_examples 'check_username when nickname is available everywhere' end - context 'and email is given' do + context 'both username and email is given' do before do xhr :get, :check_username, username: 'BruceWayne', email: 'brucie@gmail.com' end include_examples 'check_username when nickname is available everywhere' 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 shared_examples 'when email is needed to check nickname match' do diff --git a/spec/services/username_checker_service_spec.rb b/spec/services/username_checker_service_spec.rb index 9d73751018f..7f802ec6b01 100644 --- a/spec/services/username_checker_service_spec.rb +++ b/spec/services/username_checker_service_spec.rb @@ -11,10 +11,6 @@ describe UsernameCheckerService do end 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 result = @service.check_username('a', @nil_email) expect(result).to have_key(:errors) @@ -40,47 +36,75 @@ describe UsernameCheckerService do SiteSetting.stubs(:call_discourse_hub?).returns(true) end - context 'and email is given' do + context 'username and email is given' do it 'username is available locally but not globally' do - DiscourseHub.stubs(:nickname_available?).returns([false, 'suggestion']) - DiscourseHub.stubs(:nickname_match?).returns([true, false, nil]) + DiscourseHub.expects(:nickname_available?).never + 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) expected = { available: true, global_match: true } expect(result).to eq(expected) end + it 'username is available locally but not globally' do + DiscourseHub.stubs(:nickname_match?).returns([false, false, 'suggestion']) + result = @service.check_username('vincent', @email) + expected = { available: false, global_match: false, suggestion: 'suggestion' } + expect(result).to eq(expected) + end + + it 'username is available globally but not locally' do + DiscourseHub.stubs(:nickname_match?).returns([false, true, nil]) + User.stubs(:username_available?).returns(false) + UserNameSuggester.stubs(:suggest).returns('einar-j') + expected = { available: false, suggestion: 'einar-j' } + result = @service.check_username('vincent', @email) + expect(result).to eq(expected) + end + + it 'username not available anywhere' do + DiscourseHub.stubs(:nickname_match?).returns([false, false, 'suggestion']) + expected = { available: false, suggestion: 'suggestion', global_match: false } + @nil_email = nil + result = @service.check_username('vincent', @email) + expect(result).to eq(expected) + 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) + 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 - it 'username is available locally but not globally' do - 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) + context 'username is nil' do + subject(:result) { @service.check_username(nil, @email) } + include_examples "only email is given" end - it 'username is available globally but not locally' do - DiscourseHub.stubs(:nickname_match?).returns([false, true, nil]) - User.stubs(:username_available?).returns(false) - UserNameSuggester.stubs(:suggest).returns('einar-j') - expected = { available: false, suggestion: 'einar-j' } - result = @service.check_username('vincent', @email) - expect(result).to eq(expected) - end - - it 'username not available anywhere' do - DiscourseHub.stubs(:nickname_match?).returns([false, false, 'suggestion']) - expected = { available: false, suggestion: 'suggestion', global_match: false } - @nil_email = nil - result = @service.check_username('vincent', @email) - expect(result).to eq(expected) + context 'username is empty string' do + subject(:result) { @service.check_username('', @email) } + include_examples "only email is given" end end diff --git a/test/javascripts/controllers/create_account_controller_test.js b/test/javascripts/controllers/create_account_controller_test.js new file mode 100644 index 00000000000..8e6ced64cae --- /dev/null +++ b/test/javascripts/controllers/create_account_controller_test.js @@ -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'); +});