From b9c94aa234fb6198a163ca2e990a3e0b209fbf90 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Fri, 9 Jun 2017 00:40:43 +0530 Subject: [PATCH] FEATURE: add required user fields to invite accept form UX: make "accept invitation" page consistent with sign up modal --- .../controllers/create-account.js.es6 | 34 +++----------- .../discourse/controllers/invites-show.js.es6 | 27 +++++++----- .../discourse/mixins/name-validation.js.es6 | 5 +++ .../mixins/user-fields-validation.js.es6 | 35 +++++++++++++++ .../components/user-fields/confirm.hbs | 2 +- .../discourse/templates/invites/show.hbs | 22 +++++++--- app/assets/stylesheets/common/base/login.scss | 7 +++ app/assets/stylesheets/desktop/login.scss | 9 +++- app/controllers/invites_controller.rb | 4 +- app/models/invite.rb | 4 +- app/models/invite_redeemer.rb | 18 ++++++-- config/locales/client.en.yml | 1 - spec/models/invite_redeemer_spec.rb | 14 ++++++ .../invite-show-user-fields-test.js.es6 | 44 +++++++++++++++++++ 14 files changed, 171 insertions(+), 55 deletions(-) create mode 100644 app/assets/javascripts/discourse/mixins/user-fields-validation.js.es6 create mode 100644 test/javascripts/acceptance/invite-show-user-fields-test.js.es6 diff --git a/app/assets/javascripts/discourse/controllers/create-account.js.es6 b/app/assets/javascripts/discourse/controllers/create-account.js.es6 index 956b9bf35b7..9b3528f32b4 100644 --- a/app/assets/javascripts/discourse/controllers/create-account.js.es6 +++ b/app/assets/javascripts/discourse/controllers/create-account.js.es6 @@ -7,9 +7,10 @@ import InputValidation from 'discourse/models/input-validation'; import PasswordValidation from "discourse/mixins/password-validation"; import UsernameValidation from "discourse/mixins/username-validation"; import NameValidation from "discourse/mixins/name-validation"; +import UserFieldsValidation from "discourse/mixins/user-fields-validation"; import { userPath } from 'discourse/lib/url'; -export default Ember.Controller.extend(ModalFunctionality, PasswordValidation, UsernameValidation, NameValidation, { +export default Ember.Controller.extend(ModalFunctionality, PasswordValidation, UsernameValidation, NameValidation, UserFieldsValidation, { login: Ember.inject.controller(), complete: false, @@ -50,19 +51,10 @@ export default Ember.Controller.extend(ModalFunctionality, PasswordValidation, U if (this.get('emailValidation.failed')) return true; if (this.get('usernameValidation.failed')) return true; if (this.get('passwordValidation.failed')) return true; + if (this.get('userFieldsValidation.failed')) return true; - // Validate required fields - let userFields = this.get('userFields'); - if (userFields) { userFields = userFields.filterBy('field.required'); } - if (!Ember.isEmpty(userFields)) { - const anyEmpty = userFields.any(function(uf) { - const val = uf.get('value'); - return !val || Ember.isEmpty(val); - }); - if (anyEmpty) { return true; } - } return false; - }.property('passwordRequired', 'nameValidation.failed', 'emailValidation.failed', 'usernameValidation.failed', 'passwordValidation.failed', 'formSubmitted', 'userFields.@each.value'), + }.property('passwordRequired', 'nameValidation.failed', 'emailValidation.failed', 'usernameValidation.failed', 'passwordValidation.failed', 'userFieldsValidation.failed', 'formSubmitted'), usernameRequired: Ember.computed.not('authOptions.omit_username'), @@ -82,10 +74,6 @@ export default Ember.Controller.extend(ModalFunctionality, PasswordValidation, U }); }.property(), - nameInstructions: function() { - return I18n.t(Discourse.SiteSettings.full_name_required ? 'user.name.instructions_required' : 'user.name.instructions'); - }.property(), - // Check the email address emailValidation: function() { // If blank, fail without a reason @@ -212,18 +200,6 @@ export default Ember.Controller.extend(ModalFunctionality, PasswordValidation, U return self.flash(I18n.t('create_account.failed'), 'error'); }); } - }, - - _createUserFields: function() { - if (!this.site) { return; } - - let userFields = this.site.get('user_fields'); - if (userFields) { - userFields = _.sortBy(userFields, 'position').map(function(f) { - return Ember.Object.create({ value: null, field: f }); - }); - } - this.set('userFields', userFields); - }.on('init') + } }); diff --git a/app/assets/javascripts/discourse/controllers/invites-show.js.es6 b/app/assets/javascripts/discourse/controllers/invites-show.js.es6 index d032c769852..20e533ec0c5 100644 --- a/app/assets/javascripts/discourse/controllers/invites-show.js.es6 +++ b/app/assets/javascripts/discourse/controllers/invites-show.js.es6 @@ -5,15 +5,17 @@ import { ajax } from 'discourse/lib/ajax'; import PasswordValidation from "discourse/mixins/password-validation"; import UsernameValidation from "discourse/mixins/username-validation"; import NameValidation from "discourse/mixins/name-validation"; +import UserFieldsValidation from "discourse/mixins/user-fields-validation"; import { findAll as findLoginMethods } from 'discourse/models/login-method'; -export default Ember.Controller.extend(PasswordValidation, UsernameValidation, NameValidation, { +export default Ember.Controller.extend(PasswordValidation, UsernameValidation, NameValidation, UserFieldsValidation, { invitedBy: Ember.computed.alias('model.invited_by'), email: Ember.computed.alias('model.email'), accountUsername: Ember.computed.alias('model.username'), passwordRequired: Ember.computed.notEmpty('accountPassword'), successMessage: null, errorMessage: null, + userFields: null, inviteImageUrl: getUrl('/images/envelope.svg'), @computed @@ -21,11 +23,6 @@ export default Ember.Controller.extend(PasswordValidation, UsernameValidation, N return I18n.t('invites.welcome_to', {site_name: this.siteSettings.title}); }, - @computed - nameLabel() { - return I18n.t(this.siteSettings.full_name_required ? 'invites.name_label' : 'invites.name_label_optional'); - }, - @computed('email') yourEmailMessage(email) { return I18n.t('invites.your_email', {email: email}); @@ -36,20 +33,30 @@ export default Ember.Controller.extend(PasswordValidation, UsernameValidation, N return findLoginMethods(this.siteSettings, this.capabilities, this.site.isMobileDevice).length > 0; }, - @computed('usernameValidation.failed', 'passwordValidation.failed', 'nameValidation.failed') - submitDisabled(usernameFailed, passwordFailed, nameFailed) { - return usernameFailed || passwordFailed || nameFailed; + @computed('usernameValidation.failed', 'passwordValidation.failed', 'nameValidation.failed', 'userFieldsValidation.failed') + submitDisabled(usernameFailed, passwordFailed, nameFailed, userFieldsFailed) { + return usernameFailed || passwordFailed || nameFailed || userFieldsFailed; }, actions: { submit() { + + const userFields = this.get('userFields'); + let userCustomFields = {}; + if (!Ember.isEmpty(userFields)) { + userFields.forEach(function(f) { + userCustomFields[f.get('field.id')] = f.get('value'); + }); + } + ajax({ url: `/invites/show/${this.get('model.token')}.json`, type: 'PUT', data: { username: this.get('accountUsername'), name: this.get('accountName'), - password: this.get('accountPassword') + password: this.get('accountPassword'), + userCustomFields } }).then(result => { if (result.success) { diff --git a/app/assets/javascripts/discourse/mixins/name-validation.js.es6 b/app/assets/javascripts/discourse/mixins/name-validation.js.es6 index 8a286522dff..b225533c36d 100644 --- a/app/assets/javascripts/discourse/mixins/name-validation.js.es6 +++ b/app/assets/javascripts/discourse/mixins/name-validation.js.es6 @@ -3,6 +3,11 @@ import { default as computed } from 'ember-addons/ember-computed-decorators'; export default Ember.Mixin.create({ + @computed() + nameInstructions() { + return I18n.t(this.siteSettings.full_name_required ? 'user.name.instructions_required' : 'user.name.instructions'); + }, + // Validate the name. @computed('accountName') nameValidation() { diff --git a/app/assets/javascripts/discourse/mixins/user-fields-validation.js.es6 b/app/assets/javascripts/discourse/mixins/user-fields-validation.js.es6 new file mode 100644 index 00000000000..6c3f8ca3c69 --- /dev/null +++ b/app/assets/javascripts/discourse/mixins/user-fields-validation.js.es6 @@ -0,0 +1,35 @@ +import InputValidation from 'discourse/models/input-validation'; +import { on, default as computed } from 'ember-addons/ember-computed-decorators'; + +export default Ember.Mixin.create({ + + @on('init') + _createUserFields() { + if (!this.site) { return; } + + let userFields = this.site.get('user_fields'); + if (userFields) { + userFields = _.sortBy(userFields, 'position').map(function(f) { + return Ember.Object.create({ value: null, field: f }); + }); + } + this.set('userFields', userFields); + }, + + // Validate required fields + @computed('userFields.@each.value') + userFieldsValidation() { + let userFields = this.get('userFields'); + if (userFields) { userFields = userFields.filterBy('field.required'); } + if (!Ember.isEmpty(userFields)) { + const anyEmpty = userFields.any(uf => { + const val = uf.get('value'); + return !val || Ember.isEmpty(val); + }); + if (anyEmpty) { + return InputValidation.create({ failed: true }); + } + } + return InputValidation.create({ ok: true }); + } +}); diff --git a/app/assets/javascripts/discourse/templates/components/user-fields/confirm.hbs b/app/assets/javascripts/discourse/templates/components/user-fields/confirm.hbs index 5db0b43bb53..ad7f851e2c7 100644 --- a/app/assets/javascripts/discourse/templates/components/user-fields/confirm.hbs +++ b/app/assets/javascripts/discourse/templates/components/user-fields/confirm.hbs @@ -1,3 +1,3 @@
- +
diff --git a/app/assets/javascripts/discourse/templates/invites/show.hbs b/app/assets/javascripts/discourse/templates/invites/show.hbs index 5822156abd8..80eb832a63a 100644 --- a/app/assets/javascripts/discourse/templates/invites/show.hbs +++ b/app/assets/javascripts/discourse/templates/invites/show.hbs @@ -23,26 +23,36 @@

-
+ {{input value=accountUsername id="new-account-username" name="username" maxlength=maxUsernameLength autocomplete="off"}}  {{input-tip validation=usernameValidation id="username-validation"}} +
{{i18n 'user.username.instructions'}}
-
+ {{input value=accountName id="new-account-name" name="name"}} +
{{nameInstructions}}
-
+ {{password-field value=accountPassword type="password" id="new-account-password" capsLockOn=capsLockOn}}  {{input-tip validation=passwordValidation}} +
+ {{passwordInstructions}} +
{{i18n 'login.caps_lock_warning'}}
+
-
-
{{i18n 'login.caps_lock_warning'}}
-
+ {{#if userFields}} +
+ {{#each userFields as |f|}} + {{user-field field=f.field value=f.value}} + {{/each}} +
+ {{/if}} diff --git a/app/assets/stylesheets/common/base/login.scss b/app/assets/stylesheets/common/base/login.scss index 4c030178007..07ee2d93129 100644 --- a/app/assets/stylesheets/common/base/login.scss +++ b/app/assets/stylesheets/common/base/login.scss @@ -94,6 +94,13 @@ $input-width: 220px; label { font-weight: bold; } + .instructions { + color: dark-light-choose(scale-color($primary, $lightness: 50%), scale-color($secondary, $lightness: 50%)); + margin: 0; + font-size: 0.929em; + font-weight: normal; + line-height: 18px; + } } } diff --git a/app/assets/stylesheets/desktop/login.scss b/app/assets/stylesheets/desktop/login.scss index 8eafec8f207..5294cc23532 100644 --- a/app/assets/stylesheets/desktop/login.scss +++ b/app/assets/stylesheets/desktop/login.scss @@ -97,7 +97,14 @@ } } form { - label, .input { + .controls, .input { + margin-left: 20px; + margin-bottom: 10px; + }, + input, label { + margin-bottom: 0; + }, + .user-field .control-label:not(.checkbox-label) { margin-left: 20px; } } diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 1693d1616a7..0a53b075e02 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -30,12 +30,12 @@ class InvitesController < ApplicationController def perform_accept_invitation params.require(:id) - params.permit(:username, :name, :password) + params.permit(:username, :name, :password, :user_custom_fields) invite = Invite.find_by(invite_key: params[:id]) if invite.present? begin - user = invite.redeem(username: params[:username], name: params[:name], password: params[:password]) + user = invite.redeem(username: params[:username], name: params[:name], password: params[:password], user_custom_fields: params[:user_custom_fields]) if user.present? log_on_user(user) post_process_invite(user) diff --git a/app/models/invite.rb b/app/models/invite.rb index 3b76c456db3..b48b85961b9 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -52,8 +52,8 @@ class Invite < ActiveRecord::Base invalidated_at.nil? end - def redeem(username: nil, name: nil, password: nil) - InviteRedeemer.new(self, username, name, password).redeem unless expired? || destroyed? || !link_valid? + def redeem(username: nil, name: nil, password: nil, user_custom_fields: nil) + InviteRedeemer.new(self, username, name, password, user_custom_fields).redeem unless expired? || destroyed? || !link_valid? end def self.extend_permissions(topic, user, invited_by) diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index a6212f91694..3a3e304dab8 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -1,4 +1,4 @@ -InviteRedeemer = Struct.new(:invite, :username, :name, :password) do +InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_fields) do def redeem Invite.transaction do @@ -18,7 +18,7 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password) do end # extracted from User cause it is very specific to invites - def self.create_user_from_invite(invite, username, name, password=nil) + def self.create_user_from_invite(invite, username, name, password=nil, user_custom_fields=nil) user_exists = User.find_by_email(invite.email) return user if user_exists @@ -42,6 +42,18 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password) do user.approved_at = Time.zone.now end + user_fields = UserField.all + if user_custom_fields.present? && user_fields.present? + field_params = user_custom_fields || {} + fields = user.custom_fields + + user_fields.each do |f| + field_val = field_params[f.id.to_s] + fields["user_field_#{f.id}"] = field_val[0...UserField.max_length] unless field_val.blank? + end + user.custom_fields = fields + end + user.moderator = true if invite.moderator? && invite.invited_by.staff? user.save! @@ -76,7 +88,7 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password) do def get_invited_user result = get_existing_user - result ||= InviteRedeemer.create_user_from_invite(invite, username, name, password) + result ||= InviteRedeemer.create_user_from_invite(invite, username, name, password, user_custom_fields) result.send_welcome_message = false result end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index f52ca9e7382..1264bbbba78 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1101,7 +1101,6 @@ en: accept_invite: "Accept Invitation" success: "Your account has been created and you're now logged in." name_label: "Name" - name_label_optional: "Name (optional)" password_label: "Set Password (optional)" password_reset: diff --git a/spec/models/invite_redeemer_spec.rb b/spec/models/invite_redeemer_spec.rb index d1d93fda6bf..3778221dc79 100644 --- a/spec/models/invite_redeemer_spec.rb +++ b/spec/models/invite_redeemer_spec.rb @@ -93,5 +93,19 @@ describe InviteRedeemer do expect(user.confirm_password?(password)).to eq(true) expect(user.approved).to eq(true) end + + it "can set custom fields" do + required_field = Fabricate(:user_field) + optional_field= Fabricate(:user_field, required: false) + user_fields = { + required_field.id.to_s => 'value1', + optional_field.id.to_s => 'value2' + } + user = InviteRedeemer.new(invite, username, name, password, user_fields).redeem + + expect(user).to be_present + expect(user.custom_fields["user_field_#{required_field.id}"]).to eq('value1') + expect(user.custom_fields["user_field_#{optional_field.id}"]).to eq('value2') + end end end diff --git a/test/javascripts/acceptance/invite-show-user-fields-test.js.es6 b/test/javascripts/acceptance/invite-show-user-fields-test.js.es6 new file mode 100644 index 00000000000..bf4893a0e5c --- /dev/null +++ b/test/javascripts/acceptance/invite-show-user-fields-test.js.es6 @@ -0,0 +1,44 @@ +import { acceptance } from "helpers/qunit-helpers"; + +acceptance("Accept Invite - User Fields", { + site: { + user_fields: [{"id":34,"name":"I've read the terms of service","field_type":"confirm","required":true}, + {"id":35,"name":"What is your pet's name?","field_type":"text","required":true}, + {"id":36,"name":"What's your dad like?","field_type":"text","required":false}] + } +}); + +test("accept invite with user fields", () => { + visit("/invites/myvalidinvitetoken"); + andThen(() => { + ok(exists(".invites-show"), "shows the accept invite page"); + ok(exists('.user-field'), "it has at least one user field"); + ok(exists('.invites-show .btn-primary:disabled'), 'submit is disabled'); + }); + + fillIn("#new-account-name", 'John Doe'); + fillIn("#new-account-username", 'validname'); + fillIn("#new-account-password", 'secur3ty4Y0uAndMe'); + + andThen(() => { + ok(exists(".username-input .good"), "username is valid"); + ok(exists('.invites-show .btn-primary:disabled'), 'submit is still disabled due to lack of user fields'); + }); + + fillIn(".user-field input[type=text]:first", "Barky"); + + andThen(() => { + ok(exists('.invites-show .btn-primary:disabled'), 'submit is disabled because field is not checked'); + }); + + click(".user-field input[type=checkbox]"); + andThen(() => { + not(exists('.invites-show .btn-primary:disabled'), 'submit is enabled because field is checked'); + }); + + click(".user-field input[type=checkbox]"); + andThen(() => { + ok(exists('.invites-show .btn-primary:disabled'), 'unclicking the checkbox disables the submit'); + }); + +});