FEATURE: Implement new required options in admin user fields UI (#27079)

We're planning to implement a feature that allows adding required fields for existing users. This PR does some preparatory refactoring to make that possible. There should be no changes to existing behaviour. Just a small update to the admin UI.

app/assets/javascripts/admin/addon/components/admin-user-field-item.hbs

@@ -30,29 +30,51 @@
       </AdminFormRow>
     {{/if}}
 
-    <AdminFormRow @wrapLabel="true" @type="checkbox">
-      <Input @type="checkbox" @checked={{this.buffered.editable}} />
-      <span>{{i18n "admin.user_fields.editable.title"}}</span>
+    <AdminFormRow @label="admin.user_fields.requirement.title">
+      <label class="optional">
+        <RadioButton
+          @value="optional"
+          @name="optional"
+          @selection={{this.buffered.requirement}}
+        />
+        <span>{{i18n "admin.user_fields.requirement.optional.title"}}</span>
+      </label>
+
+      <label class="on_signup">
+        <RadioButton
+          @value="on_signup"
+          @name="on_signup"
+          @selection={{this.buffered.requirement}}
+        />
+        <div class="label-text">
+          <span>{{i18n "admin.user_fields.requirement.on_signup.title"}}</span>
+          <div class="description">{{i18n
+              "admin.user_fields.requirement.on_signup.description"
+            }}</div>
+        </div>
+      </label>
     </AdminFormRow>
 
-    <AdminFormRow @wrapLabel="true" @type="checkbox">
-      <Input @type="checkbox" @checked={{this.buffered.required}} />
-      <span>{{i18n "admin.user_fields.required.title"}}</span>
-    </AdminFormRow>
+    <AdminFormRow @label="admin.user_fields.preferences">
+      <label>
+        <Input @type="checkbox" @checked={{this.buffered.editable}} />
+        <span>{{i18n "admin.user_fields.editable.title"}}</span>
+      </label>
 
-    <AdminFormRow @wrapLabel="true" @type="checkbox">
-      <Input @type="checkbox" @checked={{this.buffered.show_on_profile}} />
-      <span>{{i18n "admin.user_fields.show_on_profile.title"}}</span>
-    </AdminFormRow>
+      <label>
+        <Input @type="checkbox" @checked={{this.buffered.show_on_profile}} />
+        <span>{{i18n "admin.user_fields.show_on_profile.title"}}</span>
+      </label>
 
-    <AdminFormRow @wrapLabel="true" @type="checkbox">
-      <Input @type="checkbox" @checked={{this.buffered.show_on_user_card}} />
-      <span>{{i18n "admin.user_fields.show_on_user_card.title"}}</span>
-    </AdminFormRow>
+      <label>
+        <Input @type="checkbox" @checked={{this.buffered.show_on_user_card}} />
+        <span>{{i18n "admin.user_fields.show_on_user_card.title"}}</span>
+      </label>
 
-    <AdminFormRow @wrapLabel="true" @type="checkbox">
-      <Input @type="checkbox" @checked={{this.buffered.searchable}} />
-      <span>{{i18n "admin.user_fields.searchable.title"}}</span>
+      <label>
+        <Input @type="checkbox" @checked={{this.buffered.searchable}} />
+        <span>{{i18n "admin.user_fields.searchable.title"}}</span>
+      </label>
     </AdminFormRow>
 
     <PluginOutlet

app/assets/javascripts/admin/addon/components/admin-user-field-item.js

@@ -44,16 +44,13 @@ export default Component.extend(bufferedProperty("userField"), {
   },
 
   @discourseComputed(
-    "userField.{editable,required,show_on_profile,show_on_user_card,searchable}"
+    "userField.{editable,show_on_profile,show_on_user_card,searchable}"
   )
   flags(userField) {
     const ret = [];
     if (userField.editable) {
       ret.push(I18n.t("admin.user_fields.editable.enabled"));
     }
-    if (userField.required) {
-      ret.push(I18n.t("admin.user_fields.required.enabled"));
-    }
     if (userField.show_on_profile) {
       ret.push(I18n.t("admin.user_fields.show_on_profile.enabled"));
     }
@@ -74,7 +71,7 @@ export default Component.extend(bufferedProperty("userField"), {
       "description",
       "field_type",
       "editable",
-      "required",
+      "requirement",
       "show_on_profile",
       "show_on_user_card",
       "searchable",

app/assets/stylesheets/common/admin/customize.scss

@@ -994,6 +994,22 @@ table.permalinks {
             width: 100%;
           }
         }
+        label {
+          font-weight: normal;
+          padding: 0.5em 0;
+        }
+
+        .label-text {
+          display: inline-flex;
+          flex-direction: column;
+        }
+
+        .description {
+          margin-top: 0.25em;
+          color: var(--primary-medium);
+          font-size: var(--font-down-1);
+          line-height: var(--line-height-large);
+        }
       }
       &.label-area {
         width: 25%;

app/controllers/admin/user_fields_controller.rb

@@ -7,7 +7,7 @@ class Admin::UserFieldsController < Admin::AdminController
       field_type
       editable
       description
-      required
+      requirement
       show_on_profile
       show_on_user_card
       position
@@ -20,7 +20,6 @@ class Admin::UserFieldsController < Admin::AdminController
     field = UserField.new(params.require(:user_field).permit(*Admin::UserFieldsController.columns))
 
     field.position = (UserField.maximum(:position) || 0) + 1
-    field.required = params[:user_field][:required] == "true"
     update_options(field)
 
     json_result(field, serializer: UserFieldSerializer) { field.save }

app/models/user_field.rb

@@ -2,8 +2,11 @@
 
 class UserField < ActiveRecord::Base
   include AnonCacheInvalidator
+  include HasDeprecatedColumns
   include HasSanitizableFields
 
+  deprecate_column :required, drop_from: "3.3"
+
   validates_presence_of :description, :field_type
   validates_presence_of :name, unless: -> { field_type == "confirm" }
   has_many :user_field_options, dependent: :destroy
@@ -15,10 +18,16 @@ class UserField < ActiveRecord::Base
 
   scope :public_fields, -> { where(show_on_profile: true).or(where(show_on_user_card: true)) }
 
+  enum :requirement, { optional: 0, for_all_users: 1, on_signup: 2 }.freeze
+
   def self.max_length
     2048
   end
 
+  def required?
+    !optional?
+  end
+
   def queue_index_search
     Jobs.enqueue(:index_user_fields_for_search, user_field_id: self.id)
   end
@@ -50,4 +59,5 @@ end
 #  external_name     :string
 #  external_type     :string
 #  searchable        :boolean          default(FALSE), not null
+#  requirement       :integer          default("optional"), not null
 #

app/serializers/user_field_serializer.rb

@@ -7,12 +7,17 @@ class UserFieldSerializer < ApplicationSerializer
              :field_type,
              :editable,
              :required,
+             :requirement,
              :show_on_profile,
              :show_on_user_card,
              :searchable,
              :position,
              :options
 
+  def required
+    object.required?
+  end
+
   def options
     object.user_field_options.pluck(:value)
   end

config/locales/client.en.yml

@@ -6586,6 +6586,7 @@ en:
         name: "Field Name"
         type: "Field Type"
         description: "Field Description"
+        preferences: "Preferences"
         save: "Save"
         edit: "Edit"
         delete: "Delete"
@@ -6596,6 +6597,13 @@ en:
           title: "Required at signup"
           enabled: "required"
           disabled: "not required"
+        requirement:
+          title: "Field Requirement"
+          optional:
+            title: "Optional"
+          on_signup:
+            title: "On signup"
+            description: "When new users sign up, they must fill out this field. Existing users are unaffected."
         editable:
           title: "Editable after signup"
           enabled: "editable"

db/migrate/20240520060901_add_requirement_to_user_fields.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class AddRequirementToUserFields < ActiveRecord::Migration[7.0]
+  def up
+    add_column :user_fields, :requirement, :integer, null: false, default: 0
+
+    execute <<~SQL
+      UPDATE user_fields
+      SET requirement =
+        (CASE WHEN required IS NOT TRUE THEN 0 ELSE 2 END)
+    SQL
+  end
+
+  def down
+    remove_column :user_fields, :requirement
+  end
+end

spec/fabricators/user_field_fabricator.rb

@@ -5,5 +5,5 @@ Fabricator(:user_field) do
   description "user field description"
   field_type "text"
   editable true
-  required true
+  requirement "on_signup"
 end

spec/models/user_field_spec.rb

@@ -1,6 +1,10 @@
 # frozen_string_literal: true
 
 RSpec.describe UserField do
+  it do
+    is_expected.to define_enum_for(:requirement).with_values(%w[optional for_all_users on_signup])
+  end
+
   describe "doesn't validate presence of name if field type is 'confirm'" do
     subject(:confirm_field) { described_class.new(field_type: "confirm") }
 
@@ -40,4 +44,26 @@ RSpec.describe UserField do
       job_enqueued?(job: Jobs::IndexUserFieldsForSearch, args: { user_field_id: user_field.id }),
     ).to eq(true)
   end
+
+  describe "#required?" do
+    let(:user_field) { Fabricate(:user_field, requirement:) }
+
+    context "when requirement is optional" do
+      let(:requirement) { "optional" }
+
+      it { expect(user_field).not_to be_required }
+    end
+
+    context "when requirement is for all users" do
+      let(:requirement) { "for_all_users" }
+
+      it { expect(user_field).to be_required }
+    end
+
+    context "when requirement is on signup" do
+      let(:requirement) { "on_signup" }
+
+      it { expect(user_field).to be_required }
+    end
+  end
 end

spec/requests/admin/user_fields_controller_spec.rb

@@ -17,6 +17,7 @@ RSpec.describe Admin::UserFieldsController do
                    name: "hello",
                    description: "hello desc",
                    field_type: "text",
+                   requirement: "on_signup",
                  },
                }
 
@@ -33,6 +34,7 @@ RSpec.describe Admin::UserFieldsController do
                    description: "hello desc",
                    field_type: "dropdown",
                    options: %w[a b c],
+                   requirement: "on_signup",
                  },
                }
 
@@ -162,13 +164,16 @@ RSpec.describe Admin::UserFieldsController do
                 name: "fraggle",
                 field_type: "confirm",
                 description: "muppet",
+                requirement: "optional",
               },
             }
 
         expect(response.status).to eq(200)
-        user_field.reload
-        expect(user_field.name).to eq("fraggle")
-        expect(user_field.field_type).to eq("confirm")
+        expect(user_field.reload).to have_attributes(
+          name: "fraggle",
+          field_type: "confirm",
+          required?: false,
+        )
       end
 
       it "updates the user field options" do

spec/requests/users_controller_spec.rb

@@ -1432,7 +1432,7 @@ RSpec.describe UsersController do
     context "with custom fields" do
       fab!(:user_field)
       fab!(:another_field) { Fabricate(:user_field) }
-      fab!(:optional_field) { Fabricate(:user_field, required: false) }
+      fab!(:optional_field) { Fabricate(:user_field, requirement: "optional") }
 
       context "without a value for the fields" do
         let(:create_params) do
@@ -1492,7 +1492,7 @@ RSpec.describe UsersController do
           it "value can't be nil or empty if the field is required" do
             put update_user_url, params: { user_fields: { field_id => valid_options } }
 
-            user_field.update!(required: true)
+            user_field.for_all_users!
 
             expect do
               put update_user_url, params: { user_fields: { field_id => nil } }
@@ -1506,7 +1506,7 @@ RSpec.describe UsersController do
           it "value can nil or empty if the field is not required" do
             put update_user_url, params: { user_fields: { field_id => valid_options } }
 
-            user_field.update!(required: false)
+            user_field.optional!
 
             expect do
               put update_user_url, params: { user_fields: { field_id => nil } }
@@ -1547,7 +1547,7 @@ RSpec.describe UsersController do
           it "value can't be nil if the field is required" do
             put update_user_url, params: { user_fields: { field_id => valid_options.first } }
 
-            user_field.update!(required: true)
+            user_field.for_all_users!
 
             expect do
               put update_user_url, params: { user_fields: { field_id => nil } }
@@ -1557,7 +1557,7 @@ RSpec.describe UsersController do
           it "value can be set to nil if the field is not required" do
             put update_user_url, params: { user_fields: { field_id => valid_options.last } }
 
-            user_field.update!(required: false)
+            user_field.optional!
 
             expect do
               put update_user_url, params: { user_fields: { field_id => nil } }
@@ -1614,7 +1614,7 @@ RSpec.describe UsersController do
     end
 
     context "with only optional custom fields" do
-      fab!(:user_field) { Fabricate(:user_field, required: false) }
+      fab!(:user_field) { Fabricate(:user_field, requirement: "optional") }
 
       context "without values for the fields" do
         let(:create_params) do
@@ -2378,8 +2378,8 @@ RSpec.describe UsersController do
 
         context "with user fields" do
           context "with an editable field" do
-            fab!(:user_field)
-            fab!(:optional_field) { Fabricate(:user_field, required: false) }
+            fab!(:user_field) { Fabricate(:user_field, requirement: "for_all_users") }
+            fab!(:optional_field) { Fabricate(:user_field, requirement: "optional") }
 
             it "should update the user field" do
               put "/u/#{user.username}.json",

spec/system/admin_user_fields_spec.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+describe "Admin User Fields", type: :system, js: true do
+  fab!(:current_user) { Fabricate(:admin) }
+
+  before { sign_in(current_user) }
+
+  let(:user_fields_page) { PageObjects::Pages::AdminUserFields.new }
+
+  it "correctly saves user fields" do
+    user_fields_page.visit
+    user_fields_page.add_field(name: "Occupation", description: "What you do for work")
+
+    expect(user_fields_page).to have_user_field("Occupation")
+
+    user_fields_page.refresh
+
+    expect(user_fields_page).to have_user_field("Occupation")
+  end
+
+  it "displays an error when missing required fields" do
+    user_fields_page.visit
+
+    user_fields_page.add_field(name: "Occupation", description: "")
+
+    expect(user_fields_page).to have_text(/Description can't be blank/)
+  end
+end

spec/system/page_objects/modals/signup.rb

@@ -55,6 +55,10 @@ module PageObjects
         find("#inviteCode").fill_in(with: code)
       end
 
+      def fill_custom_field(name, value)
+        find(".user-field-#{name.downcase} input").fill_in(with: value)
+      end
+
       def has_valid_email?
         find(".create-account-email").has_css?("#account-email-validation.good")
       end

spec/system/page_objects/pages/admin_user_fields.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module PageObjects
+  module Pages
+    class AdminUserFields < PageObjects::Pages::Base
+      def visit
+        page.visit "admin/customize/user_fields"
+        self
+      end
+
+      def add_field(name: nil, description: nil, requirement: nil, preferences: [])
+        page.find(".user-fields .btn-primary").click
+
+        form = page.find(".user-field")
+
+        form.find(".user-field-name").fill_in(with: name)
+        form.find(".user-field-desc").fill_in(with: description)
+        form.find(".save").click
+      end
+
+      def has_user_field?(name)
+        page.has_text?(name)
+      end
+    end
+  end
+end

spec/system/signup_spec.rb

@@ -47,6 +47,40 @@ describe "Signup", type: :system do
       end
     end
 
+    context "when there are required user fields" do
+      before do
+        Fabricate(
+          :user_field,
+          name: "Occupation",
+          requirement: "on_signup",
+          description: "What you do for work",
+        )
+      end
+
+      it "can signup when filling the custom field" do
+        signup_modal.open
+        signup_modal.fill_email("johndoe@example.com")
+        signup_modal.fill_username("john")
+        signup_modal.fill_password("supersecurepassword")
+        signup_modal.fill_custom_field("Occupation", "Jedi")
+        expect(signup_modal).to have_valid_fields
+
+        signup_modal.click_create_account
+        expect(page).to have_css(".account-created")
+      end
+
+      it "cannot signup without filling the custom field" do
+        signup_modal.open
+        signup_modal.fill_email("johndoe@example.com")
+        signup_modal.fill_username("john")
+        signup_modal.fill_password("supersecurepassword")
+
+        signup_modal.click_create_account
+        expect(signup_modal).to have_content(I18n.t("js.user_fields.required", name: "Occupation"))
+        expect(signup_modal).to have_no_css(".account-created")
+      end
+    end
+
     context "when user requires approval" do
       before do
         SiteSetting.must_approve_users = true