diff --git a/app/assets/javascripts/admin/controllers/admin-user-field-item.js.es6 b/app/assets/javascripts/admin/controllers/admin-user-field-item.js.es6 index f2ea7d746..0e0ac50be 100644 --- a/app/assets/javascripts/admin/controllers/admin-user-field-item.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-user-field-item.js.es6 @@ -9,11 +9,23 @@ export default Ember.ObjectController.extend(BufferedContent, { return UserField.fieldTypeById(this.get('field_type')).get('name'); }.property('field_type'), + flags: function() { + var ret = []; + if (this.get('editable')) { + ret.push(I18n.t('admin.user_fields.editable.enabled')); + } + if (this.get('required')) { + ret.push(I18n.t('admin.user_fields.required.enabled')); + } + + return ret.join(', '); + }.property('editable', 'required'), + actions: { save: function() { var self = this; - var attrs = this.get('buffered').getProperties('name', 'description', 'field_type', 'editable'); + var attrs = this.get('buffered').getProperties('name', 'description', 'field_type', 'editable', 'required'); this.get('model').save(attrs).then(function(res) { self.set('model.id', res.user_field.id); diff --git a/app/assets/javascripts/admin/templates/user-fields.hbs b/app/assets/javascripts/admin/templates/user-fields.hbs index bb890d75f..c829ef18c 100644 --- a/app/assets/javascripts/admin/templates/user-fields.hbs +++ b/app/assets/javascripts/admin/templates/user-fields.hbs @@ -11,12 +11,10 @@ {{input value=f.buffered.name class="user-field-name" placeholder=userFieldsName}}
- {{combo-box content=fieldTypes valueAttribute="id" value=f.buffered.field_type}} + {{input value=f.buffered.description class="user-field-desc" placeholder=userFieldsDescription}}
- + {{combo-box content=fieldTypes valueAttribute="id" value=f.buffered.field_type}}
@@ -25,29 +23,29 @@
- {{input value=f.buffered.description class="user-field-desc" placeholder=userFieldsDescription}} + +
+
+
{{else}}
-
{{f.name}}
-
{{f.fieldName}}
-
- {{#if f.editable}} - {{i18n admin.user_fields.editable.enabled}} - {{else}} - {{i18n admin.user_fields.editable.disabled}} - {{/if}} +
{{f.name}}
+
{{{f.description}}}
+
{{f.fieldName}}
+
+
+
+ + +
-
- - -
-
-
- {{f.description}} -
- +
{{f.flags}}
{{/if}}
{{/each}} diff --git a/app/assets/javascripts/discourse/controllers/create-account.js.es6 b/app/assets/javascripts/discourse/controllers/create-account.js.es6 index 6352ade30..e9798fe5e 100644 --- a/app/assets/javascripts/discourse/controllers/create-account.js.es6 +++ b/app/assets/javascripts/discourse/controllers/create-account.js.es6 @@ -51,6 +51,7 @@ export default DiscourseController.extend(ModalFunctionality, { // Validate required fields var userFields = this.get('userFields'); + if (userFields) { userFields = userFields.filterProperty('field.required'); } if (!Ember.empty(userFields)) { var anyEmpty = userFields.any(function(uf) { var val = uf.get('value'); diff --git a/app/assets/javascripts/discourse/templates/components/user-fields/text.hbs b/app/assets/javascripts/discourse/templates/components/user-fields/text.hbs index 11d2d2629..499752add 100644 --- a/app/assets/javascripts/discourse/templates/components/user-fields/text.hbs +++ b/app/assets/javascripts/discourse/templates/components/user-fields/text.hbs @@ -1,5 +1,6 @@
{{input value=value}} + {{#if field.required}}*{{/if}}

{{{field.description}}}

diff --git a/app/assets/stylesheets/common/base/login.scss b/app/assets/stylesheets/common/base/login.scss index 82d184166..5f5dc26d0 100644 --- a/app/assets/stylesheets/common/base/login.scss +++ b/app/assets/stylesheets/common/base/login.scss @@ -28,6 +28,7 @@ } .controls { margin-left: 92px; + margin-bottom: 15px; label { width: auto; text-align: left; diff --git a/app/assets/stylesheets/common/base/user.scss b/app/assets/stylesheets/common/base/user.scss index d447e4d87..910cabc5d 100644 --- a/app/assets/stylesheets/common/base/user.scss +++ b/app/assets/stylesheets/common/base/user.scss @@ -15,6 +15,15 @@ } } +.user-field { + .required { + text-align: top; + color: $danger; + font-weight: bold; + font-size: 1.3em; + } +} + .end-of-stream { border: 3px solid $primary; width: 100%; @@ -22,7 +31,7 @@ .user-navigation { - .map { + .map { height: 50px; } .avatar { diff --git a/app/assets/stylesheets/mobile/login.scss b/app/assets/stylesheets/mobile/login.scss index 8b3ab15d2..3c6275d08 100644 --- a/app/assets/stylesheets/mobile/login.scss +++ b/app/assets/stylesheets/mobile/login.scss @@ -63,6 +63,7 @@ a#forgot-password-link {clear: left; float: left; } label { width: 85px; } + input[type=text] { margin-top: 0; width: 184px; diff --git a/app/controllers/admin/user_fields_controller.rb b/app/controllers/admin/user_fields_controller.rb index 20774d923..856f48b38 100644 --- a/app/controllers/admin/user_fields_controller.rb +++ b/app/controllers/admin/user_fields_controller.rb @@ -1,7 +1,11 @@ class Admin::UserFieldsController < Admin::AdminController + def self.columns + [:name, :field_type, :editable, :description, :required] + end + def create - field = UserField.new(params.require(:user_field).permit(:name, :field_type, :editable, :description)) + field = UserField.new(params.require(:user_field).permit(*Admin::UserFieldsController.columns)) json_result(field, serializer: UserFieldSerializer) do field.save end @@ -15,10 +19,10 @@ class Admin::UserFieldsController < Admin::AdminController field_params = params.require(:user_field) field = UserField.where(id: params.require(:id)).first - field.name = field_params[:name] - field.field_type = field_params[:field_type] - field.description = field_params[:description] - field.editable = field_params[:editable] == "true" + + Admin::UserFieldsController.columns.each do |col| + field.send("#{col}=", field_params[col] || false) + end json_result(field, serializer: UserFieldSerializer) do field.save diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index ea6f338cf..4afdadbad 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -49,11 +49,12 @@ class UsersController < ApplicationController if params[:user_fields].present? params[:custom_fields] ||= {} - UserField.where(editable: true).pluck(:id).each do |fid| - val = params[:user_fields][fid.to_s] + UserField.where(editable: true).each do |f| + val = params[:user_fields][f.id.to_s] val = nil if val === "false" - return render_json_error(I18n.t("login.missing_user_field")) if val.blank? - params[:custom_fields]["user_field_#{fid}"] = val + + return render_json_error(I18n.t("login.missing_user_field")) if val.blank? && f.required? + params[:custom_fields]["user_field_#{f.id}"] = val end end @@ -188,16 +189,19 @@ class UsersController < ApplicationController user = User.new(user_params) # Handle custom fields - user_field_ids = UserField.pluck(:id) - if user_field_ids.present? - if params[:user_fields].blank? + user_fields = UserField.all + if user_fields.present? + if params[:user_fields].blank? && UserField.where(required: true).exists? return fail_with("login.missing_user_field") else fields = user.custom_fields - user_field_ids.each do |fid| - field_val = params[:user_fields][fid.to_s] - return fail_with("login.missing_user_field") if field_val.blank? - fields["user_field_#{fid}"] = field_val + user_fields.each do |f| + field_val = params[:user_fields][f.id.to_s] + if field_val.blank? + return fail_with("login.missing_user_field") if f.required? + else + fields["user_field_#{f.id}"] = field_val + end end user.custom_fields = fields end diff --git a/app/models/user_field_serializer.rb b/app/models/user_field_serializer.rb index 52a9a8b4f..ff2da805e 100644 --- a/app/models/user_field_serializer.rb +++ b/app/models/user_field_serializer.rb @@ -1,3 +1,8 @@ class UserFieldSerializer < ApplicationSerializer - attributes :id, :name, :description, :field_type, :editable + attributes :id, + :name, + :description, + :field_type, + :editable, + :required end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 25caf6d56..176704524 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2019,6 +2019,10 @@ en: delete: "Delete" cancel: "Cancel" delete_confirm: "Are you sure you want to delete that user field?" + required: + title: "Required at signup?" + enabled: "required" + disabled: "not required" editable: title: "Editable after signup?" enabled: "editable" diff --git a/db/migrate/20141008181228_add_required_signup_to_user_fields.rb b/db/migrate/20141008181228_add_required_signup_to_user_fields.rb new file mode 100644 index 000000000..d95286054 --- /dev/null +++ b/db/migrate/20141008181228_add_required_signup_to_user_fields.rb @@ -0,0 +1,5 @@ +class AddRequiredSignupToUserFields < ActiveRecord::Migration + def change + add_column :user_fields, :required, :boolean, default: true, null: false + end +end diff --git a/spec/controllers/admin/user_fields_controller_spec.rb b/spec/controllers/admin/user_fields_controller_spec.rb index 97412e742..5e6165f5a 100644 --- a/spec/controllers/admin/user_fields_controller_spec.rb +++ b/spec/controllers/admin/user_fields_controller_spec.rb @@ -32,7 +32,7 @@ describe Admin::UserFieldsController do context '.destroy' do let!(:user_field) { Fabricate(:user_field) } - it "returns a list of user fields" do + it "deletes the user field" do -> { xhr :delete, :destroy, id: user_field.id response.should be_success @@ -43,7 +43,7 @@ describe Admin::UserFieldsController do context '.update' do let!(:user_field) { Fabricate(:user_field) } - it "returns a list of user fields" do + it "updates the user field" do xhr :put, :update, id: user_field.id, user_field: {name: 'fraggle', field_type: 'confirm', description: 'muppet'} response.should be_success user_field.reload diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 6270e6bdc..1af534373 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -559,6 +559,7 @@ describe UsersController do context "with custom fields" do let!(:user_field) { Fabricate(:user_field) } let!(:another_field) { Fabricate(:user_field) } + let!(:optional_field) { Fabricate(:user_field, required: false) } context "without a value for the fields" do let(:create_params) { {name: @user.name, password: 'watwatwat', username: @user.username, email: @user.email} } @@ -577,7 +578,7 @@ describe UsersController do } } } - it "should succeed" do + it "should succeed without the optional field" do xhr :post, :create, create_params response.should be_success inserted = User.where(email: @user.email).first @@ -585,6 +586,19 @@ describe UsersController do inserted.custom_fields.should be_present inserted.custom_fields["user_field_#{user_field.id}"].should == 'value1' inserted.custom_fields["user_field_#{another_field.id}"].should == 'value2' + inserted.custom_fields["user_field_#{optional_field.id}"].should be_blank + end + + it "should succeed with the optional field" do + create_params[:user_fields][optional_field.id.to_s] = 'value3' + xhr :post, :create, create_params.merge(create_params) + response.should be_success + inserted = User.where(email: @user.email).first + inserted.should be_present + inserted.custom_fields.should be_present + inserted.custom_fields["user_field_#{user_field.id}"].should == 'value1' + inserted.custom_fields["user_field_#{another_field.id}"].should == 'value2' + inserted.custom_fields["user_field_#{optional_field.id}"].should == 'value3' end end @@ -904,6 +918,7 @@ describe UsersController do context "with user fields" do context "an editable field" do let!(:user_field) { Fabricate(:user_field) } + let!(:optional_field) { Fabricate(:user_field, required: false ) } it "should update the user field" do put :update, username: user.username, name: 'Jim Tom', user_fields: { user_field.id.to_s => 'happy' } diff --git a/spec/fabricators/user_field.rb b/spec/fabricators/user_field_fabricator.rb similarity index 90% rename from spec/fabricators/user_field.rb rename to spec/fabricators/user_field_fabricator.rb index b1a3ec3ed..41b1dff07 100644 --- a/spec/fabricators/user_field.rb +++ b/spec/fabricators/user_field_fabricator.rb @@ -3,4 +3,5 @@ Fabricator(:user_field) do description "user field description" field_type 'text' editable true + required true end diff --git a/test/javascripts/integration/create-account-user-fields-test.js.es6 b/test/javascripts/integration/create-account-user-fields-test.js.es6 index 939ea8fcc..63198c24d 100644 --- a/test/javascripts/integration/create-account-user-fields-test.js.es6 +++ b/test/javascripts/integration/create-account-user-fields-test.js.es6 @@ -2,8 +2,9 @@ import { integration } from "helpers/qunit-helpers"; integration("Create Account - User Fields", { site: { - user_fields: [{"id":34,"name":"I've read the terms of service","field_type":"confirm"}, - {"id":35,"name":"What is your pet's name?","field_type":"text"}] + 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}] } }); @@ -27,7 +28,7 @@ test("create account with user fields", function() { ok(exists('.modal-footer .btn-primary:disabled'), 'create account is still disabled due to lack of user fields'); }); - fillIn(".user-field input[type=text]", "Barky"); + fillIn(".user-field input[type=text]:first", "Barky"); andThen(function() { ok(exists('.modal-footer .btn-primary:disabled'), 'create account is disabled because field is not checked'); @@ -35,7 +36,7 @@ test("create account with user fields", function() { click(".user-field input[type=checkbox]"); andThen(function() { - not(exists('.modal-footer .btn-primary:disabled'), 'create account is disabled because field is not checked'); + not(exists('.modal-footer .btn-primary:disabled'), 'create account is enabled because field is not checked'); }); click(".user-field input[type=checkbox]");