From a3006e520457b1e7eaa7d5efd07c05f4edc24609 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Sun, 1 Mar 2026 22:40:53 +0000 Subject: [PATCH 01/23] fix user stuff --- app/controllers/users_controller.rb | 10 ++++- app/models/user.rb | 48 ++++++++++++++++++---- app/views/api/v4/users/_user.json.jbuilder | 2 + 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f3c00fd737..509c4309d1 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -253,6 +253,12 @@ def update authorize @user @user.assign_attributes(user_params) + if params[:user][:full_name].present? && params[:user][:first_name].blank? && params[:user][:last_name].blank? + require "namae" + namae = Namae.parse(params[:user][:full_name]).first + @user.first_name = (namae&.given || namae&.particle)&.split(" ")&.first + @user.last_name = namae&.family&.split(" ")&.last + end if @user.use_two_factor_authentication_changed? return unless enforce_sudo_mode # rubocop:disable Style/SoleNestedConditional @@ -309,7 +315,7 @@ def update if @user.save confetti! if !@user.seasonal_themes_enabled_before_last_save && @user.seasonal_themes_enabled? # confetti if the user enables seasonal themes - if @user.full_name_before_last_save.blank? + if @user.first_name_before_last_save.blank? flash[:success] = "Profile created!" redirect_to(return_to || root_path) else @@ -415,6 +421,8 @@ def set_onboarding def user_params attributes = [ + :first_name, + :last_name, :full_name, :preferred_name, :phone_number, diff --git a/app/models/user.rb b/app/models/user.rb index 16d3271c4c..a6146f6db8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -12,8 +12,10 @@ # comment_notifications :integer default("all_threads"), not null # creation_method :integer # email :text not null +# first_name :string # full_name :string # joined_as_teenager :boolean +# last_name :string # locked_at :datetime # payout_method_type :string # phone_number :text @@ -59,7 +61,7 @@ class User < ApplicationRecord tracked owner: proc{ |controller, record| record }, recipient: proc { |controller, record| record }, only: [:create, :update] include PgSearch::Model - pg_search_scope :search_name, against: [:full_name, :email, :phone_number], associated_against: { email_updates: :original }, using: { tsearch: { prefix: true, dictionary: "english" } } + pg_search_scope :search_name, against: [:first_name, :last_name, :email, :phone_number], associated_against: { email_updates: :original }, using: { tsearch: { prefix: true, dictionary: "english" } } friendly_id :slug_candidates, use: :slugged scope :admin, -> { where(access_level: [:admin, :superadmin]) } @@ -186,6 +188,20 @@ class User < ApplicationRecord message: "must contain your first and last name, and can't contain special characters.", allow_blank: true, } + validates :first_name, presence: true, if: -> { first_name_in_database.present? || last_name_in_database.present? } + validates :first_name, format: { + with: /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,'-]+\z/, + message: "must contain only letters and can't contain special characters.", allow_blank: true, + } + + validates :last_name, format: { + with: /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,'-]+\z/, + message: "must contain only letters and can't contain special characters.", allow_blank: true, + } + + normalizes :first_name, with: ->(name) { name.strip.presence } + normalizes :last_name, with: ->(name) { name.strip.presence } + validates :email, uniqueness: true, presence: true validates_email_format_of :email normalizes :email, with: ->(email) { email.strip.downcase } @@ -280,31 +296,45 @@ def remove_admin! end def first_name(legal: false) - @first_name ||= (namae(legal:)&.given || namae(legal:)&.particle)&.split(" ")&.first + return read_attribute(:first_name) unless legal + + read_attribute(:first_name) || (namae(legal:)&.given || namae(legal:)&.particle)&.split(" ")&.first end def last_name(legal: false) - @last_name ||= namae(legal:)&.family&.split(" ")&.last + return read_attribute(:last_name) unless legal + + read_attribute(:last_name) || namae(legal:)&.family&.split(" ")&.last + end + + def reconstructed_full_name + parts = [self[:first_name], self[:last_name]].compact + parts.empty? ? nil : parts.join(" ") end def initial_name @initial_name ||= if name.strip.split(" ").count == 1 name else - "#{(first_name || last_name)[0..20]} #{(last_name || first_name)[0, 1]}" + first = (self[:first_name] || self[:last_name] || name)[0..20] + last = (self[:last_name] || self[:first_name] || "")[0, 1] + last.blank? ? first : "#{first} #{last}" end end def safe_name # stripe requires names to be 24 chars or less, and must include a last name return name unless name.length > 24 - return full_name unless full_name.length > 24 + + reconstructed = reconstructed_full_name + return reconstructed if reconstructed && reconstructed.length <= 24 + return full_name if full_name && full_name.length <= 24 initial_name end def name - preferred_name.presence || full_name || email_handle + preferred_name.presence || reconstructed_full_name || email_handle end def possessive_name @@ -369,11 +399,11 @@ def unlock! def onboarding? # in_database to prevent a blank name update attempt from triggering onboarding. - full_name_in_database.blank? + first_name_in_database.blank? end def was_onboarding? - full_name_before_last_save.blank? + first_name_before_last_save.blank? end def active_mailbox_address @@ -467,7 +497,7 @@ def sms_charge_notifications_enabled? end def queue_sync_with_loops_job - new_user = full_name_before_last_save.blank? && !onboarding? + new_user = first_name_before_last_save.blank? && !onboarding? User::SyncUserToLoopsJob.perform_later(user_id: id, new_user:) end diff --git a/app/views/api/v4/users/_user.json.jbuilder b/app/views/api/v4/users/_user.json.jbuilder index 825adaa809..0b1dc21f1d 100644 --- a/app/views/api/v4/users/_user.json.jbuilder +++ b/app/views/api/v4/users/_user.json.jbuilder @@ -10,6 +10,8 @@ json.name user.initial_name # those which we'd like to expose less of: expand_pii(override_if: user == current_user) do json.email user.email + json.first_name user.first_name + json.last_name user.last_name json.birthday user.birthday if expand?(:shipping_address) json.shipping_address do From bc8771c5d071ef9862229afaffb7dd8c4985a4a4 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Sun, 1 Mar 2026 22:41:03 +0000 Subject: [PATCH 02/23] UI changes --- app/views/users/edit.html.erb | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 44a436fef0..e108631180 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -34,15 +34,25 @@ <%= form_errors @user, "profile", "We couldn't update your" %>
- <%= form.label :full_name %> - <%= form.text_field :full_name, required: true, placeholder: "Fiona Hackworth III", style: "max-width:100%", pattern: '^\S+.+' %> - <% @user.errors.full_messages_for(:full_name).each do |message| %> + <%= form.label :first_name %> + <%= form.text_field :first_name, required: true, placeholder: "Fiona", style: "max-width:100%" %> + <% @user.errors.full_messages_for(:first_name).each do |message| %>
<%= message %>
<% end %>
+
+ <%= form.label :last_name, "Last name (optional)" %> + <%= form.text_field :last_name, style: "max-width:100%", placeholder: "Hackworth" %> + <% @user.errors.full_messages_for(:last_name).each do |message| %> +
<%= message %>
+ <% end %> +
+
+ +
<%= form.label :preferred_name, "Preferred name (optional)" %> - <%= form.text_field :preferred_name, style: "max-width:100%", placeholder: @user.full_name.presence || "FiFi Hacks", maxlength: 30 %> + <%= form.text_field :preferred_name, style: "max-width:100%", placeholder: "FiFi Hacks", maxlength: 30 %>
@@ -137,16 +147,24 @@ <% end %>
- <%= form.label :full_name %> - <%= form.text_field :full_name, required: true, placeholder: "Fiona Hackworth III", pattern: '^\S+.+', disabled: %> - <% @user.errors.full_messages_for(:full_name).each do |message| %> + <%= form.label :first_name %> + <%= form.text_field :first_name, required: true, placeholder: "Fiona", disabled: %> + <% @user.errors.full_messages_for(:first_name).each do |message| %> +
<%= message %>
+ <% end %> +
+
+ <%= form.label :last_name %> +

Optional - you can have just a first name if you prefer.

+ <%= form.text_field :last_name, placeholder: "Hackworth", disabled: %> + <% @user.errors.full_messages_for(:last_name).each do |message| %>
<%= message %>
<% end %>
<%= form.label :preferred_name %>

If you go by a different name.

- <%= form.text_field :preferred_name, placeholder: @user.full_name.presence || "FiFi Hacks", maxlength: 30, disabled: %> + <%= form.text_field :preferred_name, placeholder: "FiFi Hacks", maxlength: 30, disabled: %>
From b6287e1c34d8f8db79980fd92165c2326d62cb3e Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Sun, 1 Mar 2026 22:41:11 +0000 Subject: [PATCH 03/23] update schema --- ...00001_add_first_and_last_names_to_users.rb | 6 +++++ ...301000002_populate_first_and_last_names.rb | 20 +++++++++++++++++ ...003_add_constraints_to_first_last_names.rb | 6 +++++ ...ove_not_null_constraint_from_first_name.rb | 5 +++++ db/schema.rb | 4 +++- spec/factories/user_factory.rb | 22 +++++++++++++++++-- 6 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20260301000001_add_first_and_last_names_to_users.rb create mode 100644 db/migrate/20260301000002_populate_first_and_last_names.rb create mode 100644 db/migrate/20260301000003_add_constraints_to_first_last_names.rb create mode 100644 db/migrate/20260301000004_remove_not_null_constraint_from_first_name.rb diff --git a/db/migrate/20260301000001_add_first_and_last_names_to_users.rb b/db/migrate/20260301000001_add_first_and_last_names_to_users.rb new file mode 100644 index 0000000000..cf6b490951 --- /dev/null +++ b/db/migrate/20260301000001_add_first_and_last_names_to_users.rb @@ -0,0 +1,6 @@ +class AddFirstAndLastNamesToUsers < ActiveRecord::Migration[8.0] + def change + add_column :users, :first_name, :string + add_column :users, :last_name, :string + end +end diff --git a/db/migrate/20260301000002_populate_first_and_last_names.rb b/db/migrate/20260301000002_populate_first_and_last_names.rb new file mode 100644 index 0000000000..4a73784598 --- /dev/null +++ b/db/migrate/20260301000002_populate_first_and_last_names.rb @@ -0,0 +1,20 @@ +class PopulateFirstAndLastNames < ActiveRecord::Migration[8.0] + def up + require 'namae' + + User.find_each do |user| + next if user.full_name.blank? + + namae = Namae.parse(user.full_name).first + + first_name_value = (namae&.given || namae&.particle)&.split(" ")&.first + last_name_value = namae&.family&.split(" ")&.last + + user.update_columns(first_name: first_name_value, last_name: last_name_value) + end + end + + def down + User.update_all(first_name: nil, last_name: nil) + end +end diff --git a/db/migrate/20260301000003_add_constraints_to_first_last_names.rb b/db/migrate/20260301000003_add_constraints_to_first_last_names.rb new file mode 100644 index 0000000000..500d2fa346 --- /dev/null +++ b/db/migrate/20260301000003_add_constraints_to_first_last_names.rb @@ -0,0 +1,6 @@ +class AddConstraintsToFirstLastNames < ActiveRecord::Migration[8.0] + def change + add_check_constraint :users, "first_name IS NOT NULL", name: "users_first_name_not_null", validate: false + # Note: last_name can be NULL to support single-name users + end +end diff --git a/db/migrate/20260301000004_remove_not_null_constraint_from_first_name.rb b/db/migrate/20260301000004_remove_not_null_constraint_from_first_name.rb new file mode 100644 index 0000000000..7b034a26e9 --- /dev/null +++ b/db/migrate/20260301000004_remove_not_null_constraint_from_first_name.rb @@ -0,0 +1,5 @@ +class RemoveNotNullConstraintFromFirstName < ActiveRecord::Migration[8.0] + def change + remove_check_constraint :users, name: "users_first_name_not_null" + end +end diff --git a/db/schema.rb b/db/schema.rb index 6d256d8f61..2cc02f6625 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_02_26_081252) do +ActiveRecord::Schema[8.0].define(version: 2026_03_01_000004) do create_schema "google_sheets" # These are extensions that must be enabled in order to support this database @@ -2623,8 +2623,10 @@ t.integer "creation_method" t.string "discord_id" t.text "email", null: false + t.string "first_name" t.string "full_name" t.boolean "joined_as_teenager" + t.string "last_name" t.datetime "locked_at", precision: nil t.bigint "payout_method_id" t.string "payout_method_type" diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 1504037ba8..89307617a3 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -2,8 +2,26 @@ FactoryBot.define do factory :user do - email { Faker::Internet.email } - full_name { Faker::Name.name } + sequence(:email) { |n| "user#{n}@example.com" } + first_name { Faker::Name.first_name } + last_name { Faker::Name.last_name } + + after(:build) do |user| + default_full_name = "#{user.first_name} #{user.last_name}" + + if user.full_name&.present? && user.full_name != default_full_name + require "namae" + namae_obj = Namae.parse(user.full_name).first + user.first_name = (namae_obj&.given || namae_obj&.particle)&.split(" ")&.first || namae_obj&.family + user.last_name = namae_obj&.family&.split(" ")&.last + elsif user.full_name.blank? + user.first_name = nil + user.last_name = nil + else + user.full_name = default_full_name + end + end + session_validity_preference { SessionsHelper::SESSION_DURATION_OPTIONS.fetch("3 days") } trait :make_admin do From 99638ef1faa143e767a7740bcfce74f01d4ff03f Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Sat, 7 Mar 2026 02:05:56 +0000 Subject: [PATCH 04/23] Remove redundant constraint migrations Instead of adding a NOT NULL constraint and then immediately removing it, we just keep the columns nullable for a smooth rollout. --- .../20260301000003_add_constraints_to_first_last_names.rb | 6 ------ ...0301000004_remove_not_null_constraint_from_first_name.rb | 5 ----- 2 files changed, 11 deletions(-) delete mode 100644 db/migrate/20260301000003_add_constraints_to_first_last_names.rb delete mode 100644 db/migrate/20260301000004_remove_not_null_constraint_from_first_name.rb diff --git a/db/migrate/20260301000003_add_constraints_to_first_last_names.rb b/db/migrate/20260301000003_add_constraints_to_first_last_names.rb deleted file mode 100644 index 500d2fa346..0000000000 --- a/db/migrate/20260301000003_add_constraints_to_first_last_names.rb +++ /dev/null @@ -1,6 +0,0 @@ -class AddConstraintsToFirstLastNames < ActiveRecord::Migration[8.0] - def change - add_check_constraint :users, "first_name IS NOT NULL", name: "users_first_name_not_null", validate: false - # Note: last_name can be NULL to support single-name users - end -end diff --git a/db/migrate/20260301000004_remove_not_null_constraint_from_first_name.rb b/db/migrate/20260301000004_remove_not_null_constraint_from_first_name.rb deleted file mode 100644 index 7b034a26e9..0000000000 --- a/db/migrate/20260301000004_remove_not_null_constraint_from_first_name.rb +++ /dev/null @@ -1,5 +0,0 @@ -class RemoveNotNullConstraintFromFirstName < ActiveRecord::Migration[8.0] - def change - remove_check_constraint :users, name: "users_first_name_not_null" - end -end From 6a0d6202e7be993750da41f446af4ccefd369720 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Sat, 7 Mar 2026 02:13:12 +0000 Subject: [PATCH 05/23] Fix nil check for user full_name in user factory --- spec/factories/user_factory.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 89307617a3..3d2c2afa8b 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -9,7 +9,7 @@ after(:build) do |user| default_full_name = "#{user.first_name} #{user.last_name}" - if user.full_name&.present? && user.full_name != default_full_name + if user.full_name.present? && user.full_name != default_full_name require "namae" namae_obj = Namae.parse(user.full_name).first user.first_name = (namae_obj&.given || namae_obj&.particle)&.split(" ")&.first || namae_obj&.family From 84f37f6dd4e095572877283efc4d5970a524a67f Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Sat, 7 Mar 2026 02:27:30 +0000 Subject: [PATCH 06/23] Refactor user name handling: replace full_name with first_name in various controllers and views --- app/controllers/logins_controller.rb | 2 +- app/controllers/stripe_cards_controller.rb | 2 +- app/controllers/users_controller.rb | 1 - app/helpers/stripe_cards_helper.rb | 2 +- app/models/user.rb | 10 +++++++++- app/services/search_service/engine/users.rb | 4 ++-- app/views/stripe_cards/_form.html.erb | 2 +- db/schema.rb | 2 +- 8 files changed, 16 insertions(+), 9 deletions(-) diff --git a/app/controllers/logins_controller.rb b/app/controllers/logins_controller.rb index 964a54ddf9..4a13998070 100644 --- a/app/controllers/logins_controller.rb +++ b/app/controllers/logins_controller.rb @@ -168,7 +168,7 @@ def complete if @login.complete? && @login.user_session.present? if @referral_link.present? redirect_to referral_link_path(@referral_link) - elsif (@user.full_name.blank? || @user.phone_number.blank?) && !@for_application + elsif (@user.first_name.blank? || @user.phone_number.blank?) && !@for_application redirect_to edit_user_path(@user.slug, return_to: params[:return_to]) elsif @login.authenticated_with_backup_code && @user.backup_codes.active.empty? redirect_to security_user_path(@user), flash: { warning: "You've just used your last backup code, and we recommend generating more." } diff --git a/app/controllers/stripe_cards_controller.rb b/app/controllers/stripe_cards_controller.rb index 8885344856..c214133b8b 100644 --- a/app/controllers/stripe_cards_controller.rb +++ b/app/controllers/stripe_cards_controller.rb @@ -188,7 +188,7 @@ def suggested(field) when :phone_number current_user.phone_number when :name - current_user.full_name + current_user.name when :line1 current_user&.stripe_cardholder&.stripe_billing_address_line1 || ecr&.last&.shipping_address_street_one diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 509c4309d1..17873ad808 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -254,7 +254,6 @@ def update @user.assign_attributes(user_params) if params[:user][:full_name].present? && params[:user][:first_name].blank? && params[:user][:last_name].blank? - require "namae" namae = Namae.parse(params[:user][:full_name]).first @user.first_name = (namae&.given || namae&.particle)&.split(" ")&.first @user.last_name = namae&.family&.split(" ")&.last diff --git a/app/helpers/stripe_cards_helper.rb b/app/helpers/stripe_cards_helper.rb index 6a817c2206..3bda3dbde4 100644 --- a/app/helpers/stripe_cards_helper.rb +++ b/app/helpers/stripe_cards_helper.rb @@ -30,7 +30,7 @@ def suggested(field) when :phone_number current_user.phone_number when :name - current_user.full_name + current_user.name when :line1 current_user&.stripe_cards&.physical&.last&.stripe_shipping_address_line1 || current_user&.stripe_cardholder&.stripe_billing_address_line1 || diff --git a/app/models/user.rb b/app/models/user.rb index 027b5daa69..e3383498e5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -169,6 +169,7 @@ class User < ApplicationRecord before_update(if: :will_save_change_to_birthday?) { self.joined_as_teenager = was_teenager_on_join? } before_create :format_number + before_save :sync_full_name_from_split_names, if: -> { first_name_changed? || last_name_changed? } before_save :on_phone_number_update validate :second_factor_present_for_2fa @@ -323,11 +324,14 @@ def initial_name end def safe_name - # stripe requires names to be 24 chars or less, and must include a last name + # stripe requires cardholder names to be 24 chars or less return name unless name.length > 24 reconstructed = reconstructed_full_name return reconstructed if reconstructed && reconstructed.length <= 24 + + # fall back to the legacy full_name which may contain a suffix that was + # stripped during migration but is still shorter than the preferred name return full_name if full_name && full_name.length <= 24 initial_name @@ -625,6 +629,10 @@ def phone_number_update_count(since:) private + def sync_full_name_from_split_names + self.full_name = reconstructed_full_name + end + def update_stripe_cardholder stripe_cardholder&.update!(stripe_email: email, stripe_phone_number: phone_number) end diff --git a/app/services/search_service/engine/users.rb b/app/services/search_service/engine/users.rb index da1c7948bb..7d003dc15e 100644 --- a/app/services/search_service/engine/users.rb +++ b/app/services/search_service/engine/users.rb @@ -14,9 +14,9 @@ def initialize(query, user, context) def run if @context[:event_id] && @query["types"].length == 1 - users = Event.find(@context[:event_id]).users.where.not(full_name: nil) + users = Event.find(@context[:event_id]).users.where.not(first_name: nil) elsif @auditor - users = User.where.not(full_name: nil) + users = User.where.not(first_name: nil) else users = User.where(id: @user.events.map { |e| e.users.pluck(:id) }.flatten) end diff --git a/app/views/stripe_cards/_form.html.erb b/app/views/stripe_cards/_form.html.erb index a64f1c7a47..35d0d39840 100644 --- a/app/views/stripe_cards/_form.html.erb +++ b/app/views/stripe_cards/_form.html.erb @@ -49,7 +49,7 @@

Shipping info

<%= form.label :stripe_shipping_name, "Shipping name" %> - <%= form.text_field :stripe_shipping_name, value: current_user&.full_name, disabled: @event&.demo_mode? %> + <%= form.text_field :stripe_shipping_name, value: current_user&.name, disabled: @event&.demo_mode? %>
<%= form.label :stripe_shipping_address_line1, "Address (line 1)" %> diff --git a/db/schema.rb b/db/schema.rb index 2cc02f6625..2caf57f0b4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2026_03_01_000004) do +ActiveRecord::Schema[8.0].define(version: 2026_03_01_000002) do create_schema "google_sheets" # These are extensions that must be enabled in order to support this database From 3bce1209a76406be842e2327a60e87377d714749 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Sat, 7 Mar 2026 03:06:11 +0000 Subject: [PATCH 07/23] Refactor user name parsing: update user factory and migration to handle first and last names more efficiently --- app/models/user.rb | 2 +- ...301000002_populate_first_and_last_names.rb | 23 +++++++++++++------ spec/controllers/logins_controller_spec.rb | 2 +- spec/factories/user_factory.rb | 9 +------- spec/misc/flipper_spec.rb | 4 ++-- spec/models/user_spec.rb | 4 ++-- 6 files changed, 23 insertions(+), 21 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index e3383498e5..607fbce2f5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -187,7 +187,7 @@ class User < ApplicationRecord validates :full_name, format: { with: /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,'-]+ [a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,' -]+\z/, message: "must contain your first and last name, and can't contain special characters.", allow_blank: true, - } + }, unless: -> { first_name_in_database.present? || last_name_in_database.present? } validates :first_name, presence: true, if: -> { first_name_in_database.present? || last_name_in_database.present? } validates :first_name, format: { diff --git a/db/migrate/20260301000002_populate_first_and_last_names.rb b/db/migrate/20260301000002_populate_first_and_last_names.rb index 4a73784598..7b77dbc7d9 100644 --- a/db/migrate/20260301000002_populate_first_and_last_names.rb +++ b/db/migrate/20260301000002_populate_first_and_last_names.rb @@ -1,17 +1,26 @@ class PopulateFirstAndLastNames < ActiveRecord::Migration[8.0] + BATCH_SIZE = 500 + def up - require 'namae' + require "namae" - User.find_each do |user| - next if user.full_name.blank? + updates = [] + User.where.not(full_name: nil).select(:id, :full_name).find_each do |user| namae = Namae.parse(user.full_name).first + updates << { + id: user.id, + first_name: (namae&.given || namae&.particle)&.split(" ")&.first, + last_name: namae&.family&.split(" ")&.last, + } - first_name_value = (namae&.given || namae&.particle)&.split(" ")&.first - last_name_value = namae&.family&.split(" ")&.last - - user.update_columns(first_name: first_name_value, last_name: last_name_value) + if updates.size >= BATCH_SIZE + User.upsert_all(updates, update_only: [:first_name, :last_name], unique_by: :id) + updates.clear + end end + + User.upsert_all(updates, update_only: [:first_name, :last_name], unique_by: :id) if updates.any? end def down diff --git a/spec/controllers/logins_controller_spec.rb b/spec/controllers/logins_controller_spec.rb index 86673c7ab8..3ee4662dcb 100644 --- a/spec/controllers/logins_controller_spec.rb +++ b/spec/controllers/logins_controller_spec.rb @@ -412,7 +412,7 @@ end it "redirects to the user's settings page if they don't have a name or phone number" do - user = create(:user, full_name: nil, phone_number: nil) + user = create(:user, first_name: nil, last_name: nil, phone_number: nil) login = create(:login, user:) login_code = create(:login_code, user:) diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 3d2c2afa8b..9f2fec7364 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -7,18 +7,11 @@ last_name { Faker::Name.last_name } after(:build) do |user| - default_full_name = "#{user.first_name} #{user.last_name}" - - if user.full_name.present? && user.full_name != default_full_name + if user.full_name.present? require "namae" namae_obj = Namae.parse(user.full_name).first user.first_name = (namae_obj&.given || namae_obj&.particle)&.split(" ")&.first || namae_obj&.family user.last_name = namae_obj&.family&.split(" ")&.last - elsif user.full_name.blank? - user.first_name = nil - user.last_name = nil - else - user.full_name = default_full_name end end diff --git a/spec/misc/flipper_spec.rb b/spec/misc/flipper_spec.rb index 7dd8e5d15e..a750a9690f 100644 --- a/spec/misc/flipper_spec.rb +++ b/spec/misc/flipper_spec.rb @@ -21,7 +21,7 @@ end it "handles users without names" do - user = create(:user, full_name: nil, email: "orpheus@hackclub.com") + user = create(:user, first_name: nil, last_name: nil, email: "orpheus@hackclub.com") actor_names = Flipper::UI.configuration.actor_names_source.call([user.flipper_id]) @@ -30,7 +30,7 @@ it "ignores unknown or no longer existent keys" do event = create(:event, name: "Hack the Planet") - user = create(:user, full_name: nil, email: "orpheus@hackclub.com") + user = create(:user, first_name: nil, last_name: nil, email: "orpheus@hackclub.com") deleted_event = create(:event) deleted_event_flipper_id = deleted_event.flipper_id diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 1555dd0bdb..53379e6f04 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -56,7 +56,7 @@ describe "#initials" do context "when missing name" do it "returns initials from email" do - user = create(:user, email: "user1@example.com", full_name: nil) + user = create(:user, email: "user1@example.com", first_name: nil, last_name: nil) expect(user.initials).to eql("U") end @@ -151,7 +151,7 @@ context "when full_name is nil" do it "returns" do - user = create(:user, email: "user1@example.com", full_name: nil) + user = create(:user, email: "user1@example.com", first_name: nil, last_name: nil) expect(user.initial_name).to eql("user1") end From 8e9cef63ffd3a079ba234a2ffeac53b384cbe6c1 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 10:56:08 -0700 Subject: [PATCH 08/23] lint --- app/controllers/users_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 2088c6cb53..ea172ececa 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -426,7 +426,7 @@ def set_onboarding def user_params attributes = [ - # account + # account :first_name, :last_name, :full_name, From 28d81202fbb13eab5ddd28e86b78ad85ec1154cd Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 13:59:55 -0700 Subject: [PATCH 09/23] Refactor name parsing logic and update user model validations for full name handling --- app/controllers/logins_controller.rb | 2 +- app/controllers/users_controller.rb | 8 +- app/models/user.rb | 41 +++--- app/services/search_service/engine/users.rb | 4 +- app/services/user_service/parse_name.rb | 55 ++++++++ ...301000002_populate_first_and_last_names.rb | 4 +- .../api/v4/card_grants_controller_spec.rb | 16 ++- spec/factories/user_factory.rb | 7 +- spec/models/user_spec.rb | 38 ++---- spec/services/user_service/parse_name_spec.rb | 128 ++++++++++++++++++ 10 files changed, 239 insertions(+), 64 deletions(-) create mode 100644 app/services/user_service/parse_name.rb create mode 100644 spec/services/user_service/parse_name_spec.rb diff --git a/app/controllers/logins_controller.rb b/app/controllers/logins_controller.rb index 87f065f82a..53ac38acbe 100644 --- a/app/controllers/logins_controller.rb +++ b/app/controllers/logins_controller.rb @@ -157,7 +157,7 @@ def complete if @login.complete? && @login.user_session.present? if @referral_link.present? redirect_to referral_link_path(@referral_link) - elsif (@user.first_name.blank? || @user.phone_number.blank?) && !@login.for_application? + elsif ((@user.first_name.blank? && @user.full_name.blank?) || @user.phone_number.blank?) && !@login.for_application? redirect_to edit_user_path(@user.slug, return_to: @login.return_to) elsif @login.authenticated_with_backup_code && @user.backup_codes.active.empty? redirect_to security_user_path(@user), flash: { warning: "You've just used your last backup code, and we recommend generating more." } diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index ea172ececa..3a28a7081d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -260,9 +260,9 @@ def update @user.assign_attributes(user_params) if params[:user][:full_name].present? && params[:user][:first_name].blank? && params[:user][:last_name].blank? - namae = Namae.parse(params[:user][:full_name]).first - @user.first_name = (namae&.given || namae&.particle)&.split(" ")&.first - @user.last_name = namae&.family&.split(" ")&.last + parsed = UserService::ParseName.new(full_name: params[:user][:full_name]).run + @user.first_name = parsed.first_name + @user.last_name = parsed.last_name end if @user.use_two_factor_authentication_changed? @@ -320,7 +320,7 @@ def update if @user.save confetti! if !@user.seasonal_themes_enabled_before_last_save && @user.seasonal_themes_enabled? # confetti if the user enables seasonal themes - if @user.first_name_before_last_save.blank? + if @user.first_name_before_last_save.blank? && @user.full_name_before_last_save.blank? flash[:success] = "Profile created!" redirect_to(return_to || root_path) else diff --git a/app/models/user.rb b/app/models/user.rb index 9cb688c705..fa393274c5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -46,6 +46,12 @@ # index_users_on_slug (slug) UNIQUE # class User < ApplicationRecord + # Character regex for name validation (supports international characters) + NAME_CHARACTERS_REGEX = /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,' -]+\z/ + + # Full name format regex (must have first and last name - at least two words) + FULL_NAME_FORMAT_REGEX = /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,'-]+ [a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,' -]+\z/ + has_paper_trail skip: [:birthday] # ciphertext columns will still be tracked include Hashid::Rails @@ -193,18 +199,18 @@ class User < ApplicationRecord validates_presence_of :birthday, if: -> { birthday_ciphertext_in_database.present? } validates :full_name, format: { - with: /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,'-]+ [a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,' -]+\z/, + with: FULL_NAME_FORMAT_REGEX, message: "must contain your first and last name, and can't contain special characters.", allow_blank: true, }, unless: -> { first_name_in_database.present? || last_name_in_database.present? } validates :first_name, presence: true, if: -> { first_name_in_database.present? || last_name_in_database.present? } validates :first_name, format: { - with: /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,'-]+\z/, + with: NAME_CHARACTERS_REGEX, message: "must contain only letters and can't contain special characters.", allow_blank: true, } validates :last_name, format: { - with: /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,'-]+\z/, + with: NAME_CHARACTERS_REGEX, message: "must contain only letters and can't contain special characters.", allow_blank: true, } @@ -305,19 +311,21 @@ def remove_admin! end def first_name(legal: false) - return read_attribute(:first_name) unless legal - - read_attribute(:first_name) || (namae(legal:)&.given || namae(legal:)&.particle)&.split(" ")&.first + # Return the stored first_name as-is. The database stores the complete first name + # (which may include particles like "von" from Namae parsing). + # For legal names, fall back to parsing full_name if first_name is not set. + read_attribute(:first_name) || (legal ? (namae(legal:)&.given || namae(legal:)&.particle)&.join(" ") : nil) end def last_name(legal: false) - return read_attribute(:last_name) unless legal - - read_attribute(:last_name) || namae(legal:)&.family&.split(" ")&.last + # Return the stored last_name as-is. The database stores the complete last name + # (which may include suffixes like "Jr." from Namae parsing). + # For legal names, fall back to parsing full_name if last_name is not set. + read_attribute(:last_name) || (legal ? namae(legal:)&.family : nil) end def reconstructed_full_name - parts = [self[:first_name], self[:last_name]].compact + parts = [self[:first_name], self[:last_name]].compact_blank parts.empty? ? nil : parts.join(" ") end @@ -325,8 +333,9 @@ def initial_name @initial_name ||= if name.strip.split(" ").count == 1 name else - first = (self[:first_name] || self[:last_name] || name)[0..20] - last = (self[:last_name] || self[:first_name] || "")[0, 1] + first = (read_attribute(:first_name)&.split(" ")&.first || read_attribute(:last_name)&.split(" ")&.first || name)[0..20] + last_source = read_attribute(:last_name) || read_attribute(:first_name) || "" + last = (last_source.split(" ").first || "")[0, 1] last.blank? ? first : "#{first} #{last}" end end @@ -346,7 +355,7 @@ def safe_name end def name - preferred_name.presence || reconstructed_full_name || email_handle + preferred_name.presence || reconstructed_full_name || full_name.presence || email_handle end def possessive_name @@ -411,11 +420,11 @@ def unlock! def onboarding? # in_database to prevent a blank name update attempt from triggering onboarding. - first_name_in_database.blank? + first_name_in_database.blank? && full_name_in_database.blank? end def was_onboarding? - first_name_before_last_save.blank? && first_name_previously_changed? + first_name_before_last_save.blank? && full_name_before_last_save.blank? && (first_name_previously_changed? || full_name_previously_changed?) end def active_mailbox_address @@ -660,7 +669,7 @@ def to_combobox_display private def sync_full_name_from_split_names - self.full_name = reconstructed_full_name + self.full_name = reconstructed_full_name if reconstructed_full_name.present? end def accessible_events(roles:) diff --git a/app/services/search_service/engine/users.rb b/app/services/search_service/engine/users.rb index 7d003dc15e..da1c7948bb 100644 --- a/app/services/search_service/engine/users.rb +++ b/app/services/search_service/engine/users.rb @@ -14,9 +14,9 @@ def initialize(query, user, context) def run if @context[:event_id] && @query["types"].length == 1 - users = Event.find(@context[:event_id]).users.where.not(first_name: nil) + users = Event.find(@context[:event_id]).users.where.not(full_name: nil) elsif @auditor - users = User.where.not(first_name: nil) + users = User.where.not(full_name: nil) else users = User.where(id: @user.events.map { |e| e.users.pluck(:id) }.flatten) end diff --git a/app/services/user_service/parse_name.rb b/app/services/user_service/parse_name.rb new file mode 100644 index 0000000000..af82dd958d --- /dev/null +++ b/app/services/user_service/parse_name.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require "namae" + +module UserService + # Parses a full name into first and last name components using the Namae gem. + # Handles complex names with particles (von, de, etc.) and suffixes (Jr., Sr., III, etc.) + class ParseName + attr_reader :given_name, :particle, :family_name, :suffix + + def initialize(full_name:) + @full_name = full_name + @given_name = nil + @particle = nil + @family_name = nil + @suffix = nil + end + + # Parse the full name and extract components + def run + return self if @full_name.blank? + + parsed = Namae.parse(@full_name).first + return self if parsed.nil? + + @given_name = parsed.given + @particle = parsed.particle + @family_name = parsed.family + @suffix = parsed.suffix + + self + end + + # Returns the formatted first name (given name + particle if present). + # Stores what should go in the database first_name column. + # Example: "von Braun" for "Wernher von Braun" + def first_name + return nil if @given_name.nil? && @family_name.nil? + + parts = [@given_name, @particle].compact_blank + return @family_name if parts.empty? + + parts.join(" ").presence + end + + # Returns the formatted last name (family name + suffix if present). + # Stores what should go in the database last_name column. + # Example: "King Jr." for "Martin Luther King Jr." + def last_name + parts = [@family_name, @suffix].compact_blank + parts.empty? ? nil : parts.join(" ").presence + end + end +end + diff --git a/db/migrate/20260301000002_populate_first_and_last_names.rb b/db/migrate/20260301000002_populate_first_and_last_names.rb index 7b77dbc7d9..1bfcd5270c 100644 --- a/db/migrate/20260301000002_populate_first_and_last_names.rb +++ b/db/migrate/20260301000002_populate_first_and_last_names.rb @@ -10,8 +10,8 @@ def up namae = Namae.parse(user.full_name).first updates << { id: user.id, - first_name: (namae&.given || namae&.particle)&.split(" ")&.first, - last_name: namae&.family&.split(" ")&.last, + first_name: [namae&.given, namae&.particle].compact_blank.join(" ").presence || namae&.family, + last_name: [namae&.family, namae&.suffix].compact_blank.join(" ").presence, } if updates.size >= BATCH_SIZE diff --git a/spec/controllers/api/v4/card_grants_controller_spec.rb b/spec/controllers/api/v4/card_grants_controller_spec.rb index f9015d45ce..40e52cfcb9 100644 --- a/spec/controllers/api/v4/card_grants_controller_spec.rb +++ b/spec/controllers/api/v4/card_grants_controller_spec.rb @@ -86,13 +86,15 @@ def card_grant_params "from" => serialized_event, "to" => serialized_event, "sender" => { - "id" => user.public_id, - "name" => "Orpheus D", - "email" => "orpheus@hackclub.com", - "admin" => false, - "auditor" => false, - "avatar" => "https://gravatar.com/avatar/stubbed", - "birthday" => nil, + "id" => user.public_id, + "name" => "Orpheus D", + "email" => "orpheus@hackclub.com", + "first_name" => "Orpheus", + "last_name" => "Dinosaur", + "admin" => false, + "auditor" => false, + "avatar" => "https://gravatar.com/avatar/stubbed", + "birthday" => nil, }, } ], diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 9f2fec7364..22f1907aa6 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -8,10 +8,9 @@ after(:build) do |user| if user.full_name.present? - require "namae" - namae_obj = Namae.parse(user.full_name).first - user.first_name = (namae_obj&.given || namae_obj&.particle)&.split(" ")&.first || namae_obj&.family - user.last_name = namae_obj&.family&.split(" ")&.last + parsed = UserService::ParseName.new(full_name: user.full_name).run + user.first_name = parsed.first_name + user.last_name = parsed.last_name end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7ca1c57d52..0a1a01ad12 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -76,7 +76,7 @@ describe "#first_name" do context "when name is downcased" do - it "returns" do + it "returns the stored first_name" do user = create(:user, full_name: "ann marie") expect(user.first_name).to eql("ann") @@ -84,31 +84,31 @@ end context "when multiple first names" do - it "returns actual first name" do + it "returns the parsed first name value" do user = create(:user, full_name: "Prof. Donald Ervin Knuth") - expect(user.first_name).to eql("Donald") + expect(user.first_name).to eql("Donald Ervin") end end context "when name entered with comma" do - it "returns actual first name" do + it "returns the parsed first name" do user = create(:user, full_name: "Turing, Alan M.") - expect(user.first_name).to eql("Alan") + expect(user.first_name).to eql("Alan M.") end end end describe "#last_name" do - it "returns actual last name" do + it "returns the stored last name with suffix" do user = create(:user, full_name: "Ken Griffey Jr.") - expect(user.last_name).to eql("Griffey") + expect(user.last_name).to eql("Griffey Jr.") end context "when name is downcased" do - it "returns" do + it "returns the stored last_name" do user = create(:user, full_name: "ann marie") expect(user.last_name).to eql("marie") @@ -116,10 +116,10 @@ end context "when entered with comma" do - it "returns actual last name" do + it "returns the parsed last name" do user = create(:user, full_name: "Carreño Quiñones, María-Jose") - expect(user.last_name).to eql("Quiñones") + expect(user.last_name).to eql("Carreño Quiñones") end end end @@ -131,24 +131,6 @@ expect(user.initial_name).to eql("First L") end - context "when first name is missing" do - it "is invalid" do - user = build(:user, full_name: "Last") - - expect(user).not_to be_valid - expect(user.errors[:full_name]).not_to be_empty - end - end - - context "when last name is missing" do - it "is invalid" do - user = build(:user, full_name: "First") - - expect(user).not_to be_valid - expect(user.errors[:full_name]).not_to be_empty - end - end - context "when full_name is nil" do it "returns" do user = create(:user, email: "user1@example.com", first_name: nil, last_name: nil) diff --git a/spec/services/user_service/parse_name_spec.rb b/spec/services/user_service/parse_name_spec.rb new file mode 100644 index 0000000000..f3db9a29b4 --- /dev/null +++ b/spec/services/user_service/parse_name_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe UserService::ParseName do + describe "#run" do + it "returns self for chaining" do + service = described_class.new(full_name: "John Doe").run + expect(service).to be_a(described_class) + end + + it "handles blank full_name" do + service = described_class.new(full_name: "").run + expect(service.first_name).to be_nil + expect(service.last_name).to be_nil + end + + it "handles nil full_name" do + service = described_class.new(full_name: nil).run + expect(service.first_name).to be_nil + expect(service.last_name).to be_nil + end + end + + describe "#first_name" do + it "parses simple names" do + service = described_class.new(full_name: "John Doe").run + expect(service.first_name).to eq("John") + end + + it "keeps middle names in first_name" do + service = described_class.new(full_name: "Martin Luther King").run + expect(service.first_name).to eq("Martin Luther") + end + + it "handles names with particles (von, de, etc.)" do + service = described_class.new(full_name: "Wernher von Braun").run + expect(service.first_name).to eq("Wernher von") + end + + it "uses family name as fallback when given name is blank" do + # Edge case where Namae only extracts family + service = described_class.new(full_name: "Madonna").run + expect(service.first_name).to eq("Madonna") + end + end + + describe "#last_name" do + it "parses simple names" do + service = described_class.new(full_name: "John Doe").run + expect(service.last_name).to eq("Doe") + end + + it "includes suffixes in last_name" do + service = described_class.new(full_name: "Martin Luther King Jr.").run + expect(service.last_name).to eq("King Jr.") + end + + it "handles multiple word last names" do + service = described_class.new(full_name: "José María García García").run + # Depending on Namae parsing, family might be "García García" or just "García" + expect(service.last_name).not_to be_nil + end + end + + describe "international names" do + it "handles accented characters" do + service = described_class.new(full_name: "José María Martínez").run + expect(service.first_name).not_to be_nil + expect(service.last_name).not_to be_nil + end + + it "preserves unicode characters in parsing" do + service = described_class.new(full_name: "François Müller").run + expect(service.first_name).not_to be_nil + expect(service.last_name).not_to be_nil + end + + it "handles hyphenated first and last names" do + service = described_class.new(full_name: "Jean-Claude Van Damme").run + expect(service.first_name).not_to be_nil + expect(service.last_name).not_to be_nil + end + end + + describe "composite names" do + it "doesn't fail on single names" do + service = described_class.new(full_name: "Madonna").run + expect(service.first_name).to eq("Madonna") + expect(service.last_name).to be_nil + end + + it "handles comma-separated names" do + service = described_class.new(full_name: "Doe, John").run + # Namae can parse this format + expect(service.first_name).not_to be_nil + expect(service.last_name).not_to be_nil + end + + it "handles titles in names" do + service = described_class.new(full_name: "Prof. Donald Ervin Knuth").run + # Namae strips titles - should work fine + expect(service.first_name).not_to be_nil + expect(service.last_name).not_to be_nil + end + end + + describe "component accessors" do + let(:parsed) { described_class.new(full_name: "Martin Luther King Jr.").run } + + it "exposes given_name" do + expect(parsed.given_name).to eq("Martin Luther") + end + + it "exposes family_name" do + expect(parsed.family_name).to eq("King") + end + + it "exposes particle" do + expect(parsed.particle).to be_nil + end + + it "exposes suffix" do + expect(parsed.suffix).to eq("Jr.") + end + end +end + From 6088847bff1ba1e71b26a32766f74ae8145d3f61 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 14:00:47 -0700 Subject: [PATCH 10/23] lint --- app/models/user.rb | 4 ++-- app/services/user_service/parse_name.rb | 2 +- spec/services/user_service/parse_name_spec.rb | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index fa393274c5..d34a9ad4d8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -333,8 +333,8 @@ def initial_name @initial_name ||= if name.strip.split(" ").count == 1 name else - first = (read_attribute(:first_name)&.split(" ")&.first || read_attribute(:last_name)&.split(" ")&.first || name)[0..20] - last_source = read_attribute(:last_name) || read_attribute(:first_name) || "" + first = (self[:first_name]&.split(" ")&.first || self[:last_name]&.split(" ")&.first || name)[0..20] + last_source = self[:last_name] || self[:first_name] || "" last = (last_source.split(" ").first || "")[0, 1] last.blank? ? first : "#{first} #{last}" end diff --git a/app/services/user_service/parse_name.rb b/app/services/user_service/parse_name.rb index af82dd958d..96cb371662 100644 --- a/app/services/user_service/parse_name.rb +++ b/app/services/user_service/parse_name.rb @@ -50,6 +50,6 @@ def last_name parts = [@family_name, @suffix].compact_blank parts.empty? ? nil : parts.join(" ").presence end + end end - diff --git a/spec/services/user_service/parse_name_spec.rb b/spec/services/user_service/parse_name_spec.rb index f3db9a29b4..c114b2ec13 100644 --- a/spec/services/user_service/parse_name_spec.rb +++ b/spec/services/user_service/parse_name_spec.rb @@ -125,4 +125,3 @@ end end end - From b3e1bd809d87f4a433f0e84c61febfbb7e48dd18 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 19:34:29 -0700 Subject: [PATCH 11/23] Update first name in card grant response to include full name --- spec/controllers/api/v4/card_grants_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/api/v4/card_grants_controller_spec.rb b/spec/controllers/api/v4/card_grants_controller_spec.rb index 40e52cfcb9..9a53a4b918 100644 --- a/spec/controllers/api/v4/card_grants_controller_spec.rb +++ b/spec/controllers/api/v4/card_grants_controller_spec.rb @@ -89,7 +89,7 @@ def card_grant_params "id" => user.public_id, "name" => "Orpheus D", "email" => "orpheus@hackclub.com", - "first_name" => "Orpheus", + "first_name" => "Orpheus the", "last_name" => "Dinosaur", "admin" => false, "auditor" => false, From 0dd64bfb59d8eec79bfb25d52b8262b4dd2fb83f Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 19:56:06 -0700 Subject: [PATCH 12/23] Update spec/factories/user_factory.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- spec/factories/user_factory.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 22f1907aa6..c2b38f1e72 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -7,10 +7,10 @@ last_name { Faker::Name.last_name } after(:build) do |user| - if user.full_name.present? + if user.full_name.present? && (user.first_name.blank? || user.last_name.blank?) parsed = UserService::ParseName.new(full_name: user.full_name).run - user.first_name = parsed.first_name - user.last_name = parsed.last_name + user.first_name = parsed.first_name if user.first_name.blank? + user.last_name = parsed.last_name if user.last_name.blank? end end From ca726151c8281557ce1d3fcf7009570136e65bc3 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 19:56:14 -0700 Subject: [PATCH 13/23] Update db/migrate/20260301000002_populate_first_and_last_names.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- db/migrate/20260301000002_populate_first_and_last_names.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20260301000002_populate_first_and_last_names.rb b/db/migrate/20260301000002_populate_first_and_last_names.rb index 1bfcd5270c..1172102800 100644 --- a/db/migrate/20260301000002_populate_first_and_last_names.rb +++ b/db/migrate/20260301000002_populate_first_and_last_names.rb @@ -15,12 +15,12 @@ def up } if updates.size >= BATCH_SIZE - User.upsert_all(updates, update_only: [:first_name, :last_name], unique_by: :id) + User.upsert_all(updates, update_only: [:first_name, :last_name]) updates.clear end end - User.upsert_all(updates, update_only: [:first_name, :last_name], unique_by: :id) if updates.any? + User.upsert_all(updates, update_only: [:first_name, :last_name]) if updates.any? end def down From 6785ba4f7e8a4198319825543111b9338260a52c Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 19:56:28 -0700 Subject: [PATCH 14/23] Update app/services/user_service/parse_name.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/services/user_service/parse_name.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/user_service/parse_name.rb b/app/services/user_service/parse_name.rb index 96cb371662..a3e85499a3 100644 --- a/app/services/user_service/parse_name.rb +++ b/app/services/user_service/parse_name.rb @@ -33,7 +33,7 @@ def run # Returns the formatted first name (given name + particle if present). # Stores what should go in the database first_name column. - # Example: "von Braun" for "Wernher von Braun" + # Example: "Wernher von" for "Wernher von Braun" def first_name return nil if @given_name.nil? && @family_name.nil? From 073f493d30372b1952e0e4c4932438b73982043b Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 19:56:36 -0700 Subject: [PATCH 15/23] Update app/models/user.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/models/user.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index d34a9ad4d8..7b047827a4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -200,18 +200,18 @@ class User < ApplicationRecord validates :full_name, format: { with: FULL_NAME_FORMAT_REGEX, - message: "must contain your first and last name, and can't contain special characters.", allow_blank: true, + message: "must contain your first and last name and may only include letters, spaces, hyphens, apostrophes, periods, and commas.", allow_blank: true, }, unless: -> { first_name_in_database.present? || last_name_in_database.present? } validates :first_name, presence: true, if: -> { first_name_in_database.present? || last_name_in_database.present? } validates :first_name, format: { with: NAME_CHARACTERS_REGEX, - message: "must contain only letters and can't contain special characters.", allow_blank: true, + message: "may only include letters, spaces, hyphens, apostrophes, periods, and commas.", allow_blank: true, } validates :last_name, format: { with: NAME_CHARACTERS_REGEX, - message: "must contain only letters and can't contain special characters.", allow_blank: true, + message: "may only include letters, spaces, hyphens, apostrophes, periods, and commas.", allow_blank: true, } normalizes :first_name, with: ->(name) { name.strip.presence } From 812492e4fa317b3143e21982569af2f68954e5b5 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 19:56:43 -0700 Subject: [PATCH 16/23] Update app/models/user.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 7b047827a4..9dd7a0c6a0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -201,7 +201,7 @@ class User < ApplicationRecord validates :full_name, format: { with: FULL_NAME_FORMAT_REGEX, message: "must contain your first and last name and may only include letters, spaces, hyphens, apostrophes, periods, and commas.", allow_blank: true, - }, unless: -> { first_name_in_database.present? || last_name_in_database.present? } + }, unless: -> { first_name.present? || last_name.present? } validates :first_name, presence: true, if: -> { first_name_in_database.present? || last_name_in_database.present? } validates :first_name, format: { From c39e5ab851176f41ef52fc3b5f394e9d63e6b8c3 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 19:56:53 -0700 Subject: [PATCH 17/23] Update app/models/user.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/models/user.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 9dd7a0c6a0..a21c085ae3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -314,7 +314,12 @@ def first_name(legal: false) # Return the stored first_name as-is. The database stores the complete first name # (which may include particles like "von" from Namae parsing). # For legal names, fall back to parsing full_name if first_name is not set. - read_attribute(:first_name) || (legal ? (namae(legal:)&.given || namae(legal:)&.particle)&.join(" ") : nil) + return read_attribute(:first_name) if read_attribute(:first_name) + return nil unless legal + + parsed_name = namae(legal:) + parts = [parsed_name&.given, parsed_name&.particle].compact_blank + parts.empty? ? nil : parts.join(" ") end def last_name(legal: false) From 580e7038a15589e016cc63611806a94f066ef9fc Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Fri, 17 Apr 2026 19:57:06 -0700 Subject: [PATCH 18/23] Update app/services/user_service/parse_name.rb Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- app/services/user_service/parse_name.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/services/user_service/parse_name.rb b/app/services/user_service/parse_name.rb index a3e85499a3..3eaa17ced7 100644 --- a/app/services/user_service/parse_name.rb +++ b/app/services/user_service/parse_name.rb @@ -51,5 +51,6 @@ def last_name parts.empty? ? nil : parts.join(" ").presence end + end end From 568cd67db881fc17cd9215a92dad05a0c36fc304 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Mon, 2 Nov 2026 14:22:02 -0800 Subject: [PATCH 19/23] revive pr --- .../event/applications_controller.rb | 2 +- app/controllers/logins_controller.rb | 2 +- app/controllers/stripe_cards_controller.rb | 2 +- app/controllers/users/first_controller.rb | 2 +- app/controllers/users_controller.rb | 8 +- app/helpers/stripe_cards_helper.rb | 2 +- app/models/stripe_cardholder.rb | 15 +- app/models/user.rb | 145 ++++---- app/services/user_service/parse_name.rb | 65 ++-- app/views/api/v4/users/_user.json.jbuilder | 4 +- app/views/event/applications/edit.html.erb | 15 +- .../event/applications/personal_info.html.erb | 15 +- app/views/stripe_cards/_form.html.erb | 2 +- app/views/users/edit.html.erb | 48 +-- app/views/users/first/new.html.erb | 12 +- config/locales/en.yml | 2 + ...00001_add_first_and_last_names_to_users.rb | 6 - ...301000002_populate_first_and_last_names.rb | 29 -- ...00001_add_first_and_last_names_to_users.rb | 11 + ...0002_backfill_user_first_and_last_names.rb | 39 +++ db/schema.rb | 2 +- spec/controllers/users_controller_spec.rb | 92 +++++ spec/factories/user_factory.rb | 16 +- spec/models/stripe_cardholder_spec.rb | 53 +++ spec/models/user_spec.rb | 317 +++++++++++++----- spec/requests/users/first_controller_spec.rb | 17 +- spec/services/user_service/parse_name_spec.rb | 150 +++------ 27 files changed, 686 insertions(+), 387 deletions(-) delete mode 100644 db/migrate/20260301000001_add_first_and_last_names_to_users.rb delete mode 100644 db/migrate/20260301000002_populate_first_and_last_names.rb create mode 100644 db/migrate/20261102000001_add_first_and_last_names_to_users.rb create mode 100644 db/migrate/20261102000002_backfill_user_first_and_last_names.rb diff --git a/app/controllers/event/applications_controller.rb b/app/controllers/event/applications_controller.rb index eb4ba3a8d8..3d108c3906 100644 --- a/app/controllers/event/applications_controller.rb +++ b/app/controllers/event/applications_controller.rb @@ -320,7 +320,7 @@ def application_params end def user_params - params.require(:event_application).permit(:full_name, :preferred_name, :phone_number, :birthday) + params.require(:event_application).permit(:first_name, :last_name, :preferred_name, :phone_number, :birthday) end def record_pageview diff --git a/app/controllers/logins_controller.rb b/app/controllers/logins_controller.rb index 16fe376c85..e888551fb6 100644 --- a/app/controllers/logins_controller.rb +++ b/app/controllers/logins_controller.rb @@ -194,7 +194,7 @@ def complete redirect_to first_index_path elsif @referral_link.present? redirect_to referral_link_path(@referral_link) - elsif ((@user.first_name.blank? && @user.full_name.blank?) || @user.phone_number.blank?) && !@login.for_application? + elsif (@user.full_name.blank? || @user.phone_number.blank?) && !@login.for_application? redirect_to edit_user_path(@user.slug, return_to: @login.return_to) elsif @login.authenticated_with_backup_code && @user.backup_codes.active.empty? redirect_to security_user_path(@user), flash: { warning: "You've just used your last backup code, and we recommend generating more." } diff --git a/app/controllers/stripe_cards_controller.rb b/app/controllers/stripe_cards_controller.rb index c734965aaa..f0c4831182 100644 --- a/app/controllers/stripe_cards_controller.rb +++ b/app/controllers/stripe_cards_controller.rb @@ -204,7 +204,7 @@ def suggested(field) when :phone_number current_user.phone_number when :name - current_user.name + current_user.full_name when :line1 current_user&.stripe_cardholder&.stripe_billing_address_line1 || ecr&.last&.shipping_address_street_one diff --git a/app/controllers/users/first_controller.rb b/app/controllers/users/first_controller.rb index 35059928ee..8b2822821e 100644 --- a/app/controllers/users/first_controller.rb +++ b/app/controllers/users/first_controller.rb @@ -189,7 +189,7 @@ def macbook_qr_code private def user_params - params.require(:user).permit(:email, :full_name, affiliations_attributes: [:league, :team_number, :name, :team_name, :role]) + params.require(:user).permit(:email, :first_name, :last_name, affiliations_attributes: [:league, :team_number, :name, :team_name, :role]) end def user_ref_params diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5fece16981..6183bece5d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -379,11 +379,6 @@ def update authorize @user @user.assign_attributes(user_params) - if params[:user][:full_name].present? && params[:user][:first_name].blank? && params[:user][:last_name].blank? - parsed = UserService::ParseName.new(full_name: params[:user][:full_name]).run - @user.first_name = parsed.first_name - @user.last_name = parsed.last_name - end payout_method_type = params.dig(:user, :payout_method_type) @@ -465,7 +460,7 @@ def update if saved confetti! if !@user.seasonal_themes_enabled_before_last_save && @user.seasonal_themes_enabled? # confetti if the user enables seasonal themes - if @user.first_name_before_last_save.blank? && @user.full_name_before_last_save.blank? + if @user.full_name_before_last_save.blank? flash[:success] = "Profile created!" redirect_to(return_to || root_path) else @@ -576,7 +571,6 @@ def user_params # account :first_name, :last_name, - :full_name, :preferred_name, :phone_number, :birthday, diff --git a/app/helpers/stripe_cards_helper.rb b/app/helpers/stripe_cards_helper.rb index 3bda3dbde4..6a817c2206 100644 --- a/app/helpers/stripe_cards_helper.rb +++ b/app/helpers/stripe_cards_helper.rb @@ -30,7 +30,7 @@ def suggested(field) when :phone_number current_user.phone_number when :name - current_user.name + current_user.full_name when :line1 current_user&.stripe_cards&.physical&.last&.stripe_shipping_address_line1 || current_user&.stripe_cardholder&.stripe_billing_address_line1 || diff --git a/app/models/stripe_cardholder.rb b/app/models/stripe_cardholder.rb index b8376b1a84..4cb265cb19 100644 --- a/app/models/stripe_cardholder.rb +++ b/app/models/stripe_cardholder.rb @@ -128,15 +128,22 @@ def reset_billing_address_to_default! end def self.first_name(user) - clean_name(user.first_name(legal: true)) + clean_name(user.first_name(legal: true), field: "first name") end def self.last_name(user) - clean_name(user.last_name(legal: true)) + clean_name(user.last_name(legal: true), field: "last name") end - def self.clean_name(name) - name = ActiveSupport::Inflector.transliterate(name || "") + def self.clean_name(name, field: "name") + # Stripe Issuing needs both names, but HCB lets people go by a first name + # alone. Say so plainly instead of letting the character check below + # blame "special characters" for a name that was never entered. + if name.blank? + raise Errors::StripeInvalidNameError, "A legal #{field} is required to issue cards. Please add it in your account settings." + end + + name = ActiveSupport::Inflector.transliterate(name) # Remove invalid characters requirements = <<~REQ.squish diff --git a/app/models/user.rb b/app/models/user.rb index 07b585f754..bc878fb6ea 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,11 +50,10 @@ # index_users_on_slug (slug) UNIQUE # class User < ApplicationRecord - # Character regex for name validation (supports international characters) + # Latin letters (including common accented ones) plus the punctuation that + # legitimately appears in names. Stripe Issuing only accepts latin names. NAME_CHARACTERS_REGEX = /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,' -]+\z/ - - # Full name format regex (must have first and last name - at least two words) - FULL_NAME_FORMAT_REGEX = /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,'-]+ [a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,' -]+\z/ + NAME_CHARACTERS_MESSAGE = "may only contain letters, spaces, hyphens, apostrophes, periods, and commas." has_paper_trail skip: [:birthday] # ciphertext columns will still be tracked @@ -79,7 +78,7 @@ class User < ApplicationRecord tracked owner: proc{ |controller, record| record }, recipient: proc { |controller, record| record }, only: [:create, :update] include PgSearch::Model - pg_search_scope :search_name, against: [:id, :first_name, :last_name, :full_name, :email, :phone_number], associated_against: { email_updates: :original }, using: { tsearch: { prefix: true, dictionary: "english" } } + pg_search_scope :search_name, against: [:id, :full_name, :email, :phone_number], associated_against: { email_updates: :original }, using: { tsearch: { prefix: true, dictionary: "english" } } friendly_id :slug_candidates, use: :slugged scope :admin, -> { where(access_level: [:admin, :superadmin]) } @@ -212,7 +211,11 @@ class User < ApplicationRecord before_save :clear_pretend_is_not_admin, if: -> { pretend_is_not_admin? && !admin_override_pretend? } before_create :format_number - before_save :sync_full_name_from_split_names, if: -> { first_name_changed? || last_name_changed? } + # `first_name` / `last_name` are the source of truth and `full_name` is kept + # in sync from them. Legacy writers that assign `full_name` directly (older + # clients, scripts) have it split into the two columns instead. + before_validation :split_full_name, if: -> { full_name_changed? && !first_name_changed? && !last_name_changed? } + before_validation :sync_full_name, if: -> { first_name_changed? || last_name_changed? } before_save :on_phone_number_update validate :second_factor_present_for_2fa @@ -231,27 +234,23 @@ class User < ApplicationRecord before_update :set_default_seasonal_theme - validates_presence_of :full_name, if: -> { full_name_in_database.present? } validates_presence_of :birthday, if: -> { birthday_ciphertext_in_database.present? } - validates :full_name, format: { - with: FULL_NAME_FORMAT_REGEX, - message: "must contain your first and last name and may only include letters, spaces, hyphens, apostrophes, periods, and commas.", allow_blank: true, - }, unless: -> { first_name.present? || last_name.present? } - - validates :first_name, presence: true, if: -> { first_name_in_database.present? || last_name_in_database.present? } - validates :first_name, format: { - with: NAME_CHARACTERS_REGEX, - message: "may only include letters, spaces, hyphens, apostrophes, periods, and commas.", allow_blank: true, - } + # Users are created with just an email and add their name during onboarding, + # so a first name is only required once there is a last name to go with it + # or once a name has been set (it can be changed, but not removed). + validates :first_name, presence: true, if: -> { self[:last_name].present? || clearing_existing_name? } + validates :first_name, format: { with: NAME_CHARACTERS_REGEX, message: NAME_CHARACTERS_MESSAGE, allow_blank: true } + validates :last_name, format: { with: NAME_CHARACTERS_REGEX, message: NAME_CHARACTERS_MESSAGE, allow_blank: true } - validates :last_name, format: { - with: NAME_CHARACTERS_REGEX, - message: "may only include letters, spaces, hyphens, apostrophes, periods, and commas.", allow_blank: true, - } + normalizes :first_name, with: ->(name) { name.squish.presence } + normalizes :last_name, with: ->(name) { name.squish.presence } - normalizes :first_name, with: ->(name) { name.strip.presence } - normalizes :last_name, with: ->(name) { name.strip.presence } + # Validate the stored columns, not the `first_name` / `last_name` readers + # below (which favor the preferred name and fall back to `full_name`). + def read_attribute_for_validation(attribute) + attribute.to_s.in?(%w[first_name last_name]) ? self[attribute] : super + end validates :email, uniqueness: true, presence: true validates_email_format_of :email @@ -357,57 +356,46 @@ def remove_admin! user! end + # The user's first name as stored, which may include middle names and + # particles ("Donald Ervin", "Wernher von"). + # + # Without `legal: true` a preferred name takes precedence, so greetings and + # other display uses call people what they asked to be called. Pass + # `legal: true` for anything that must match the user's legal name (Stripe). def first_name(legal: false) - # Return the stored first_name as-is. The database stores the complete first name - # (which may include particles like "von" from Namae parsing). - # For legal names, fall back to parsing full_name if first_name is not set. - return read_attribute(:first_name) if read_attribute(:first_name) - return nil unless legal + return preferred_name_parts.first if !legal && preferred_name.present? - parsed_name = namae(legal:) - parts = [parsed_name&.given, parsed_name&.particle].compact_blank - parts.empty? ? nil : parts.join(" ") + self[:first_name] || legacy_parsed_name.first_name end + # The user's last name as stored, which may include a suffix ("King Jr."). + # Returns nil for users who only have a first name, and for single-word + # preferred names when `legal` is false. def last_name(legal: false) - # Return the stored last_name as-is. The database stores the complete last name - # (which may include suffixes like "Jr." from Namae parsing). - # For legal names, fall back to parsing full_name if last_name is not set. - read_attribute(:last_name) || (legal ? namae(legal:)&.family : nil) - end + return preferred_name_parts.last if !legal && preferred_name.present? - def reconstructed_full_name - parts = [self[:first_name], self[:last_name]].compact_blank - parts.empty? ? nil : parts.join(" ") + self[:last_name] || legacy_parsed_name.last_name end + # "Fiona H" — what we show to other users. def initial_name - @initial_name ||= if name.strip.split(" ").count == 1 - name - else - first = (self[:first_name]&.split(" ")&.first || self[:last_name]&.split(" ")&.first || name)[0..20] - last_source = self[:last_name] || self[:first_name] || "" - last = (last_source.split(" ").first || "")[0, 1] - last.blank? ? first : "#{first} #{last}" - end + first = first_name + last = last_name + return name if first.blank? || last.blank? + + "#{first.split(" ").first[0..20]} #{last[0]}" end def safe_name # stripe requires cardholder names to be 24 chars or less return name unless name.length > 24 - - reconstructed = reconstructed_full_name - return reconstructed if reconstructed && reconstructed.length <= 24 - - # fall back to the legacy full_name which may contain a suffix that was - # stripped during migration but is still shorter than the preferred name - return full_name if full_name && full_name.length <= 24 + return full_name if full_name.present? && full_name.length <= 24 initial_name end def name - preferred_name.presence || reconstructed_full_name || full_name.presence || email_handle + preferred_name.presence || full_name.presence || email_handle end def possessive_name @@ -518,11 +506,11 @@ def unlock! def onboarding? # in_database to prevent a blank name update attempt from triggering onboarding. - first_name_in_database.blank? && full_name_in_database.blank? + full_name_in_database.blank? end def was_onboarding? - first_name_before_last_save.blank? && full_name_before_last_save.blank? && (first_name_previously_changed? || full_name_previously_changed?) + full_name_before_last_save.blank? && full_name_previously_changed? end def active_mailbox_address @@ -781,8 +769,35 @@ def onboarding_contractor_positions private - def sync_full_name_from_split_names - self.full_name = reconstructed_full_name if reconstructed_full_name.present? + def split_full_name + parsed = UserService::ParseName.new(full_name:).run + self.first_name = parsed.first_name + self.last_name = parsed.last_name + end + + def sync_full_name + self.full_name = [self[:first_name], self[:last_name]].compact_blank.join(" ").presence + end + + # A name can be changed but not removed once set. Rows that haven't been + # backfilled yet (`full_name` set, `first_name` nil) are only held to this + # when the name itself is being edited, so unrelated updates still save. + def clearing_existing_name? + full_name_in_database.present? && (first_name_changed? || last_name_changed? || full_name_changed?) + end + + # Preferred names are free-form ("FiFi", "FiFi Hacks"), so split them + # positionally: everything but the last word is the first name. + def preferred_name_parts + words = preferred_name.squish.split(" ") + return [words.first, nil] if words.one? + + [words[0..-2].join(" "), words.last] + end + + # Rows written before the split columns existed and not yet backfilled. + def legacy_parsed_name + UserService::ParseName.new(full_name:).run end def create_legal_entity @@ -814,18 +829,6 @@ def update_stripe_cardholder ) end - def namae(legal: false) - if legal - @legal_namae ||= Namae.parse(full_name).first - else - @namae ||= Namae.parse(name).first || Namae.parse(name_simplified).first || Namae::Name.new(given: name_simplified) - end - end - - def name_simplified - name.split(/[^[[:word:]]]+/).join(" ") - end - def email_handle @email_handle ||= email.split("@").first end diff --git a/app/services/user_service/parse_name.rb b/app/services/user_service/parse_name.rb index 3eaa17ced7..0c9e5dca7b 100644 --- a/app/services/user_service/parse_name.rb +++ b/app/services/user_service/parse_name.rb @@ -3,54 +3,55 @@ require "namae" module UserService - # Parses a full name into first and last name components using the Namae gem. - # Handles complex names with particles (von, de, etc.) and suffixes (Jr., Sr., III, etc.) + # Splits a single-field full name into the first and last name stored on + # `users.first_name` / `users.last_name`. + # + # Only used where a full name still arrives as one string: the backfill of + # rows that predate the split columns, and legacy writers that assign + # `User#full_name` directly. Everything else should set the two columns. + # + # Namae handles comma forms ("Doe, John"), particles ("Wernher von Braun") + # and suffixes ("King Jr."). It also keeps a list of honorifics, and a + # surname that collides with one ("Jane Elder", "Mary Lord") makes it either + # give up entirely or treat the surname as a title. In those cases we fall + # back to a positional split so a real surname is never lost. class ParseName - attr_reader :given_name, :particle, :family_name, :suffix + Result = Struct.new(:first_name, :last_name, keyword_init: true) def initialize(full_name:) - @full_name = full_name - @given_name = nil - @particle = nil - @family_name = nil - @suffix = nil + @full_name = full_name.to_s.squish end - # Parse the full name and extract components def run - return self if @full_name.blank? + return Result.new(first_name: nil, last_name: nil) if @full_name.blank? parsed = Namae.parse(@full_name).first - return self if parsed.nil? + return positional_split if parsed.nil? || trailing_word_taken_as_title?(parsed) - @given_name = parsed.given - @particle = parsed.particle - @family_name = parsed.family - @suffix = parsed.suffix + first = [parsed.given, parsed.particle].compact_blank.join(" ").presence + last = [parsed.family, parsed.suffix].compact_blank.join(" ").presence - self - end + # "Madonna" parses as given-only; "Mr. Smith" parses as family-only. + # Either way the one word we have belongs in first_name. + first, last = last, nil if first.nil? - # Returns the formatted first name (given name + particle if present). - # Stores what should go in the database first_name column. - # Example: "Wernher von" for "Wernher von Braun" - def first_name - return nil if @given_name.nil? && @family_name.nil? + Result.new(first_name: first, last_name: last) + end - parts = [@given_name, @particle].compact_blank - return @family_name if parts.empty? + private - parts.join(" ").presence + # Namae parsed "Jane W Elder" as given "Jane", family "W", title "Elder". + # An honorific at the very end of a name is a surname, not a title. + def trailing_word_taken_as_title?(parsed) + parsed.title.present? && @full_name.downcase.end_with?(parsed.title.downcase) end - # Returns the formatted last name (family name + suffix if present). - # Stores what should go in the database last_name column. - # Example: "King Jr." for "Martin Luther King Jr." - def last_name - parts = [@family_name, @suffix].compact_blank - parts.empty? ? nil : parts.join(" ").presence - end + def positional_split + words = @full_name.split(" ") + return Result.new(first_name: words.first, last_name: nil) if words.one? + Result.new(first_name: words[0..-2].join(" "), last_name: words.last) + end end end diff --git a/app/views/api/v4/users/_user.json.jbuilder b/app/views/api/v4/users/_user.json.jbuilder index fc9ad49e59..8649584d5c 100644 --- a/app/views/api/v4/users/_user.json.jbuilder +++ b/app/views/api/v4/users/_user.json.jbuilder @@ -12,8 +12,8 @@ object_shape(json, user, created_at: false) do # those which we'd like to expose less of: expand_pii(override_if: user == current_user || local_assigns[:show_email]) do json.email user.email - json.first_name user.first_name - json.last_name user.last_name + json.first_name user.first_name(legal: true) + json.last_name user.last_name(legal: true) end expand_pii(override_if: user == current_user) do diff --git a/app/views/event/applications/edit.html.erb b/app/views/event/applications/edit.html.erb index e717eec433..21aefdfd26 100644 --- a/app/views/event/applications/edit.html.erb +++ b/app/views/event/applications/edit.html.erb @@ -116,17 +116,22 @@
- <%= form.label :full_name do %> - Full name * + <%= form.label :first_name do %> + Legal first name * <% end %> - <%= form.text_field :full_name, value: @application.user.full_name.presence, required: true, placeholder: "Fiona Hackworth III", style: "max-width:100%", pattern: '^\S+.+' %> + <%= form.text_field :first_name, value: @application.user.first_name(legal: true).presence, required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %>
- <%= form.label :preferred_name, "Preferred name (optional)" %> - <%= form.text_field :preferred_name, value: @application.user.preferred_name.presence, style: "max-width:100%", placeholder: "FiFi Hacks", maxlength: 30 %> + <%= form.label :last_name, "Legal last name (optional)" %> + <%= form.text_field :last_name, value: @application.user.last_name(legal: true).presence, placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %>
+
+ <%= form.label :preferred_name, "Preferred name (optional)" %> + <%= form.text_field :preferred_name, value: @application.user.preferred_name.presence, style: "max-width:100%", placeholder: "FiFi Hacks", maxlength: 30, autocomplete: "nickname" %> +
+
<%= form.label :phone_number do %> diff --git a/app/views/event/applications/personal_info.html.erb b/app/views/event/applications/personal_info.html.erb index 64ea184b39..bf03fc6083 100644 --- a/app/views/event/applications/personal_info.html.erb +++ b/app/views/event/applications/personal_info.html.erb @@ -17,17 +17,22 @@
- <%= form.label :full_name do %> - Full name * + <%= form.label :first_name do %> + Legal first name * <% end %> - <%= form.text_field :full_name, value: @application.user.full_name.presence, required: true, placeholder: "Fiona Hackworth III", style: "max-width:100%", pattern: '^\S+.+' %> + <%= form.text_field :first_name, value: @application.user.first_name(legal: true).presence, required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %>
- <%= form.label :preferred_name, "Preferred name (optional)" %> - <%= form.text_field :preferred_name, value: @application.user.preferred_name.presence, style: "max-width:100%", placeholder: "FiFi Hacks", maxlength: 30 %> + <%= form.label :last_name, "Legal last name (optional)" %> + <%= form.text_field :last_name, value: @application.user.last_name(legal: true).presence, placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %>
+
+ <%= form.label :preferred_name, "Preferred name (optional)" %> + <%= form.text_field :preferred_name, value: @application.user.preferred_name.presence, style: "max-width:100%", placeholder: "FiFi Hacks", maxlength: 30, autocomplete: "nickname" %> +
+
<%= form.label :phone_number do %> diff --git a/app/views/stripe_cards/_form.html.erb b/app/views/stripe_cards/_form.html.erb index 556342488f..5461dc9279 100644 --- a/app/views/stripe_cards/_form.html.erb +++ b/app/views/stripe_cards/_form.html.erb @@ -55,7 +55,7 @@

Shipping info

<%= form.label :stripe_shipping_name, "Shipping name" %> - <%= form.text_field :stripe_shipping_name, value: current_user&.name, disabled: @event&.demo_mode? %> + <%= form.text_field :stripe_shipping_name, value: current_user&.full_name, disabled: @event&.demo_mode? %>
<%= form.label :stripe_shipping_address_line1, "Address (line 1)" %> diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 6c54e266f5..fa94f2569a 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -35,25 +35,24 @@
<%= form.label :first_name %> - <%= form.text_field :first_name, required: true, placeholder: "Fiona", style: "max-width:100%" %> + <%= form.text_field :first_name, value: @user.first_name(legal: true), required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %> <% @user.errors.full_messages_for(:first_name).each do |message| %>
<%= message %>
<% end %>
- <%= form.label :last_name, "Last name (optional)" %> - <%= form.text_field :last_name, style: "max-width:100%", placeholder: "Hackworth" %> + <%= form.label :last_name, "Legal last name (optional)" %> + <%= form.text_field :last_name, value: @user.last_name(legal: true), placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %> <% @user.errors.full_messages_for(:last_name).each do |message| %>
<%= message %>
<% end %>
+

Your legal name is used for cards and payouts. Leave the last name blank if you only have one name.

-
-
- <%= form.label :preferred_name, "Preferred name (optional)" %> - <%= form.text_field :preferred_name, style: "max-width:100%", placeholder: "FiFi Hacks", maxlength: 30 %> -
+
+ <%= form.label :preferred_name, "Preferred name (optional)" %> + <%= form.text_field :preferred_name, style: "max-width:100%", placeholder: "FiFi Hacks", maxlength: 30, autocomplete: "nickname" %>
@@ -147,25 +146,28 @@
<% end %> -
- <%= form.label :first_name %> - <%= form.text_field :first_name, required: true, placeholder: "Fiona", disabled: %> - <% @user.errors.full_messages_for(:first_name).each do |message| %> -
<%= message %>
- <% end %> -
-
- <%= form.label :last_name %> -

Optional - you can have just a first name if you prefer.

- <%= form.text_field :last_name, placeholder: "Hackworth", disabled: %> - <% @user.errors.full_messages_for(:last_name).each do |message| %> -
<%= message %>
- <% end %> +
+
+ <%= form.label :first_name %> +

As it appears on your ID. Used for cards and payouts.

+ <%= form.text_field :first_name, value: @user.first_name(legal: true), required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name", disabled: %> + <% @user.errors.full_messages_for(:first_name).each do |message| %> +
<%= message %>
+ <% end %> +
+
+ <%= form.label :last_name %> +

Optional if you only have one name.

+ <%= form.text_field :last_name, value: @user.last_name(legal: true), placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name", disabled: %> + <% @user.errors.full_messages_for(:last_name).each do |message| %> +
<%= message %>
+ <% end %> +
<%= form.label :preferred_name %>

If you go by a different name.

- <%= form.text_field :preferred_name, placeholder: "FiFi Hacks", maxlength: 30, disabled: %> + <%= form.text_field :preferred_name, placeholder: "FiFi Hacks", maxlength: 30, autocomplete: "nickname", disabled: %>
diff --git a/app/views/users/first/new.html.erb b/app/views/users/first/new.html.erb index e83fdbc01f..5a80154f52 100644 --- a/app/views/users/first/new.html.erb +++ b/app/views/users/first/new.html.erb @@ -34,9 +34,15 @@ About you

Tell us a little about yourself.

-
- <%= form.label :full_name, "Full name" %> - <%= form.text_field :full_name, placeholder: "Fiona Hackworth" %> +
+
+ <%= form.label :first_name, "First name" %> + <%= form.text_field :first_name, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %> +
+
+ <%= form.label :last_name, "Last name" %> + <%= form.text_field :last_name, placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %> +
<%= form.label :email, "Email address" %> diff --git a/config/locales/en.yml b/config/locales/en.yml index c8f98a3b8a..4dc0ee69e7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -39,6 +39,8 @@ en: slug: "URL" user: full_name: "Legal name" + first_name: "Legal first name" + last_name: "Legal last name" errors: models: donation: diff --git a/db/migrate/20260301000001_add_first_and_last_names_to_users.rb b/db/migrate/20260301000001_add_first_and_last_names_to_users.rb deleted file mode 100644 index cf6b490951..0000000000 --- a/db/migrate/20260301000001_add_first_and_last_names_to_users.rb +++ /dev/null @@ -1,6 +0,0 @@ -class AddFirstAndLastNamesToUsers < ActiveRecord::Migration[8.0] - def change - add_column :users, :first_name, :string - add_column :users, :last_name, :string - end -end diff --git a/db/migrate/20260301000002_populate_first_and_last_names.rb b/db/migrate/20260301000002_populate_first_and_last_names.rb deleted file mode 100644 index 1172102800..0000000000 --- a/db/migrate/20260301000002_populate_first_and_last_names.rb +++ /dev/null @@ -1,29 +0,0 @@ -class PopulateFirstAndLastNames < ActiveRecord::Migration[8.0] - BATCH_SIZE = 500 - - def up - require "namae" - - updates = [] - - User.where.not(full_name: nil).select(:id, :full_name).find_each do |user| - namae = Namae.parse(user.full_name).first - updates << { - id: user.id, - first_name: [namae&.given, namae&.particle].compact_blank.join(" ").presence || namae&.family, - last_name: [namae&.family, namae&.suffix].compact_blank.join(" ").presence, - } - - if updates.size >= BATCH_SIZE - User.upsert_all(updates, update_only: [:first_name, :last_name]) - updates.clear - end - end - - User.upsert_all(updates, update_only: [:first_name, :last_name]) if updates.any? - end - - def down - User.update_all(first_name: nil, last_name: nil) - end -end diff --git a/db/migrate/20261102000001_add_first_and_last_names_to_users.rb b/db/migrate/20261102000001_add_first_and_last_names_to_users.rb new file mode 100644 index 0000000000..54adf6edac --- /dev/null +++ b/db/migrate/20261102000001_add_first_and_last_names_to_users.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddFirstAndLastNamesToUsers < ActiveRecord::Migration[8.1] + def change + change_table :users, bulk: true do |t| + t.string :first_name + t.string :last_name + end + end + +end diff --git a/db/migrate/20261102000002_backfill_user_first_and_last_names.rb b/db/migrate/20261102000002_backfill_user_first_and_last_names.rb new file mode 100644 index 0000000000..c93c35adc7 --- /dev/null +++ b/db/migrate/20261102000002_backfill_user_first_and_last_names.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# Splits every existing `users.full_name` into `first_name` / `last_name`. +# Rows that already have a first name are skipped, so this is safe to re-run. +class BackfillUserFirstAndLastNames < ActiveRecord::Migration[8.1] + disable_ddl_transaction! + + BATCH_SIZE = 1_000 + + class MigrationUser < ApplicationRecord + self.table_name = "users" + + end + + def up + MigrationUser.where(first_name: nil).where.not(full_name: [nil, ""]).in_batches(of: BATCH_SIZE) do |batch| + rows = batch.pluck(:id, :full_name).filter_map do |id, full_name| + parsed = UserService::ParseName.new(full_name:).run + next if parsed.first_name.blank? + + [id, parsed.first_name, parsed.last_name] + end + next if rows.empty? + + values = rows.map { |row| MigrationUser.sanitize_sql_array(["(?, ?, ?)", *row]) }.join(", ") + MigrationUser.connection.execute(<<~SQL.squish) + UPDATE users + SET first_name = parsed.first_name, last_name = parsed.last_name + FROM (VALUES #{values}) AS parsed(id, first_name, last_name) + WHERE users.id = parsed.id + SQL + end + end + + def down + # The columns are dropped by the previous migration; nothing to undo here. + end + +end diff --git a/db/schema.rb b/db/schema.rb index 1f5515f329..aa2dad0964 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_10_02_120100) do +ActiveRecord::Schema[8.1].define(version: 2026_11_02_000002) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_catalog.plpgsql" diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 463f352c1c..8e04c8be58 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -201,6 +201,33 @@ end end + describe "#edit" do + render_views + + it "asks a new user for their first and last name during onboarding" do + user = create(:user, first_name: nil, last_name: nil) + create_session(user, verified: true) + + get(:edit, params: { id: user.id }) + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Create your account") + expect(response.body).to include('name="user[first_name]"') + expect(response.body).to include('name="user[last_name]"') + expect(response.body).not_to include('name="user[full_name]"') + end + + it "shows the stored legal name in settings, not the preferred name" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi") + create_session(user, verified: true) + + get(:edit, params: { id: user.id }) + + expect(response.body).to include('name="user[first_name]"').and include('value="Fiona"') + expect(response.body).to include('name="user[last_name]"').and include('value="Hackworth"') + end + end + describe "#update" do render_views @@ -328,6 +355,71 @@ expect(user.reload.default_payout_method).to be_nil end + it "completes onboarding once the user enters their name" do + user = create(:user, first_name: nil, last_name: nil) + create_session(user, verified: true) + + patch(:update, params: { id: user.id, user: { first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi" } }) + + expect(response).to redirect_to(root_path) + expect(flash[:success]).to eq("Profile created!") + expect(user.reload.attributes).to include("first_name" => "Fiona", "last_name" => "Hackworth", "full_name" => "Fiona Hackworth", "preferred_name" => "FiFi") + expect(user).not_to be_onboarding + end + + it "lets a user go by a first name only" do + user = create(:user, first_name: nil, last_name: nil) + create_session(user, verified: true) + + patch(:update, params: { id: user.id, user: { first_name: "Madonna", last_name: "" } }) + + expect(response).to redirect_to(root_path) + expect(user.reload).to have_attributes(first_name: "Madonna", last_name: nil, full_name: "Madonna") + end + + it "updates the user's name" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + create_session(user, verified: true) + + patch(:update, params: { id: user.id, user: { first_name: "Jane", last_name: "Elder" } }) + + expect(response).to redirect_to(edit_user_path(user)) + expect(flash[:success]).to eq("Updated your profile!") + expect(user.reload).to have_attributes(first_name: "Jane", last_name: "Elder", full_name: "Jane Elder") + end + + it "does not let the first name be removed" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + create_session(user, verified: true) + + patch(:update, params: { id: user.id, user: { first_name: "", last_name: "Hackworth" } }) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include("Legal first name can't be blank") + expect(user.reload.full_name).to eq("Fiona Hackworth") + end + + it "rejects names with unsupported characters" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + create_session(user, verified: true) + + patch(:update, params: { id: user.id, user: { first_name: "Fiona", last_name: "Hackworth [Dev]" } }) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include("Legal last name may only contain letters") + expect(user.reload.last_name).to eq("Hackworth") + end + + it "ignores the legacy full_name parameter" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + create_session(user, verified: true) + + patch(:update, params: { id: user.id, user: { full_name: "Someone Else" } }) + + expect(response).to have_http_status(:found) + expect(user.reload).to have_attributes(first_name: "Fiona", last_name: "Hackworth", full_name: "Fiona Hackworth") + end + it "lets an admin bypass phone number verification for a user" do admin_user = create(:user, :make_admin) user = create(:user, phone_number: "+18556254225") diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 2d84c2037f..191bbb3d0a 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -3,17 +3,11 @@ FactoryBot.define do factory :user do sequence(:email) { |n| "user#{n}@example.com" } - first_name { Faker::Name.first_name } - last_name { Faker::Name.last_name } - - after(:build) do |user| - if user.full_name.present? && (user.first_name.blank? || user.last_name.blank?) - parsed = UserService::ParseName.new(full_name: user.full_name).run - user.first_name = parsed.first_name if user.first_name.blank? - user.last_name = parsed.last_name if user.last_name.blank? - end - end - + # Passing `full_name:` is still supported: the model splits it into + # `first_name` / `last_name` on validation, so the defaults step aside. + full_name { nil } + first_name { full_name.present? ? nil : Faker::Name.first_name } + last_name { full_name.present? ? nil : Faker::Name.last_name } session_validity_preference { SessionsHelper::SESSION_DURATION_OPTIONS.fetch("3 days") } verified { true } diff --git a/spec/models/stripe_cardholder_spec.rb b/spec/models/stripe_cardholder_spec.rb index 6d806538db..f7938e5fd9 100644 --- a/spec/models/stripe_cardholder_spec.rb +++ b/spec/models/stripe_cardholder_spec.rb @@ -83,6 +83,59 @@ expect(stripe_cardholder.reload.stripe_phone_number).to be_nil end + describe ".first_name and .last_name" do + it "use the user's legal name, not their preferred name" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi Hacks") + + expect(StripeCardholder.first_name(user)).to eq("Fiona") + expect(StripeCardholder.last_name(user)).to eq("Hackworth") + end + + it "keep middle names and suffixes" do + user = create(:user, first_name: "Martin Luther", last_name: "King Jr.") + + expect(StripeCardholder.first_name(user)).to eq("Martin Luther") + expect(StripeCardholder.last_name(user)).to eq("King Jr.") + end + + it "work for surnames that collide with Namae's title list" do + user = create(:user, full_name: "Jane Elder") + + expect(StripeCardholder.first_name(user)).to eq("Jane") + expect(StripeCardholder.last_name(user)).to eq("Elder") + end + + it "transliterate accented characters" do + user = create(:user, first_name: "María-José", last_name: "Carreño Quiñones") + + expect(StripeCardholder.first_name(user)).to eq("Maria-Jose") + expect(StripeCardholder.last_name(user)).to eq("Carreno Quinones") + end + + it "explain that a last name is required when the user only has a first name" do + user = create(:user, first_name: "Madonna", last_name: nil) + + expect(StripeCardholder.first_name(user)).to eq("Madonna") + expect { StripeCardholder.last_name(user) }.to raise_error( + Errors::StripeInvalidNameError, + "A legal last name is required to issue cards. Please add it in your account settings." + ) + end + + it "explain that a first name is required when the user has no name" do + user = create(:user, first_name: nil, last_name: nil) + + expect { StripeCardholder.first_name(user) }.to raise_error(Errors::StripeInvalidNameError, /legal first name is required/) + end + + it "reject names with no latin letters" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user.update_columns(first_name: "ジョン", full_name: "ジョン Hackworth") + + expect { StripeCardholder.first_name(user) }.to raise_error(Errors::StripeInvalidNameError, /must contain at least 1 letter/) + end + end + describe ".phone_number_supported?" do it "works for US numbers" do expect(StripeCardholder.phone_number_supported?("+18556254225")).to eq(true) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index fafa334e8f..3536efe5fc 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -96,13 +96,168 @@ end end - context "when full_name is removed" do - it "fails validation" do + describe "name storage" do + it "keeps full_name in sync with first_name and last_name" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + expect(user.full_name).to eq("Fiona Hackworth") + + user.update!(last_name: "Hackworth-Smith") + expect(user.reload.full_name).to eq("Fiona Hackworth-Smith") + end + + it "stores a first name on its own" do + user = create(:user, first_name: "Madonna", last_name: nil) + + expect(user.full_name).to eq("Madonna") + expect(user.last_name).to be_nil + end + + it "splits a full_name assigned by a legacy writer" do + user = create(:user, full_name: "Martin Luther King Jr.") + + expect(user.first_name).to eq("Martin Luther") + expect(user.last_name).to eq("King Jr.") + expect(user.full_name).to eq("Martin Luther King Jr.") + end + + it "canonicalizes a comma-form full_name" do + user = create(:user, full_name: "Doe, John") + + expect(user.reload).to have_attributes(first_name: "John", last_name: "Doe", full_name: "John Doe") + end + + it "re-splits when a legacy writer changes full_name" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + + user.update!(full_name: "Jane Elder") + + expect(user.reload).to have_attributes(first_name: "Jane", last_name: "Elder", full_name: "Jane Elder") + end + + it "prefers the split names when both are assigned at once" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + + user.update!(first_name: "Jane", last_name: "Elder", full_name: "Someone Else") + + expect(user.reload).to have_attributes(first_name: "Jane", last_name: "Elder", full_name: "Jane Elder") + end + + it "trims and collapses whitespace" do + user = create(:user, first_name: " Fiona Rose ", last_name: " Hackworth ") + + expect(user).to have_attributes(first_name: "Fiona Rose", last_name: "Hackworth", full_name: "Fiona Rose Hackworth") + end + + it "treats a blank last name as no last name" do + user = create(:user, first_name: "Fiona", last_name: " ") + + expect(user.last_name).to be_nil + expect(user.full_name).to eq("Fiona") + end + + it "updates the personal legal entity's name" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + expect(user.personal_legal_entity.name).to eq("Fiona Hackworth") + + user.update!(first_name: "Jane", last_name: "Elder") + + expect(user.personal_legal_entity.reload.name).to eq("Jane Elder") + end + + context "for a row that predates the split columns" do + let(:user) do + create(:user, first_name: "Fiona", last_name: "Hackworth").tap do |u| + u.update_columns(first_name: nil, last_name: nil, full_name: "Jane Elder") + u.reload + end + end + + it "derives the names from full_name" do + expect(user.first_name).to eq("Jane") + expect(user.last_name).to eq("Elder") + expect(user.first_name(legal: true)).to eq("Jane") + expect(user.initial_name).to eq("Jane E") + end + + it "still allows unrelated updates without touching the name" do + expect(user.update(phone_number: "+18556254225")).to be(true) + expect(user.reload.first_name_in_database).to be_nil + expect(user.full_name).to eq("Jane Elder") + end + + it "does not allow the name to be removed" do + user.update(first_name: "", last_name: "") + + expect(user.reload.full_name).to eq("Jane Elder") + end + + it "backfills the columns once the name is edited" do + user.update!(first_name: "Jane", last_name: "Elder") + + expect(user.reload).to have_attributes(first_name: "Jane", last_name: "Elder", full_name: "Jane Elder") + end + end + end + + describe "name validations" do + it "allows a user without a name" do + expect(build(:user, first_name: nil, last_name: nil)).to be_valid + end + + it "requires a first name when there is a last name" do + user = build(:user, first_name: nil, last_name: "Hackworth") + + expect(user).not_to be_valid + expect(user.errors[:first_name]).to include("can't be blank") + end + + it "does not allow a name to be removed once set" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + + expect(user.update(first_name: "", last_name: "")).to be(false) + expect(user.errors[:first_name]).to include("can't be blank") + expect(user.reload.full_name).to eq("Fiona Hackworth") + end + + it "does not allow a name to be removed through full_name either" do user = create(:user, full_name: "Caleb Denio") - expect(user.full_name).to eq("Caleb Denio") - user.update(full_name: "") - expect(user).to_not be_valid + expect(user.update(full_name: "")).to be(false) + expect(user.errors[:first_name]).to include("can't be blank") + end + + it "allows the last name to be removed" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + + expect(user.update(last_name: "")).to be(true) + expect(user.reload.full_name).to eq("Fiona") + end + + it "accepts accented letters and name punctuation" do + user = build(:user, first_name: "María-José", last_name: "O'Brien, Jr.") + + expect(user).to be_valid + end + + { + "brackets" => ["Zach", "Latta [Dev]"], + "parentheses" => ["Max (test)", "Wofford"], + "emoji" => ["Melody ✨", nil], + "digits" => ["5512700050241863", nil], + }.each do |label, (first, last)| + it "rejects #{label}" do + user = build(:user, first_name: first, last_name: last) + + expect(user).not_to be_valid + expect(user.errors[:first_name] + user.errors[:last_name]).to include(User::NAME_CHARACTERS_MESSAGE) + end + end + + it "reports invalid characters from a legacy full_name on the split fields" do + user = build(:user, full_name: "Zach Latta [Dev]") + + expect(user).not_to be_valid + expect(user.errors[:last_name]).to include(User::NAME_CHARACTERS_MESSAGE) end end @@ -117,6 +272,18 @@ end describe "#safe_name" do + it "returns the preferred name when it fits" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi") + + expect(user.safe_name).to eql("FiFi") + end + + it "falls back to the legal name when the preferred name is too long" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "Fiona the Magnificent Hacker") + + expect(user.safe_name).to eql("Fiona Hackworth") + end + context "when initial name is really long" do it "returns safe_name max of 24 chars" do user = create(:user, full_name: "Thisisareallyreallylongfirstnamethatembursewillnotlike Last") @@ -127,65 +294,81 @@ end end - describe "#first_name" do - context "when name is downcased" do - it "returns the stored first_name" do - user = create(:user, full_name: "ann marie") + describe "#first_name and #last_name" do + it "return the stored names" do + user = create(:user, first_name: "Donald Ervin", last_name: "Knuth") - expect(user.first_name).to eql("ann") - end + expect(user.first_name).to eql("Donald Ervin") + expect(user.last_name).to eql("Knuth") + expect(user.first_name(legal: true)).to eql("Donald Ervin") + expect(user.last_name(legal: true)).to eql("Knuth") end - context "when multiple first names" do - it "returns the parsed first name value" do - user = create(:user, full_name: "Prof. Donald Ervin Knuth") + it "return nil for a user without a name" do + user = create(:user, first_name: nil, last_name: nil) - expect(user.first_name).to eql("Donald Ervin") - end + expect(user.first_name).to be_nil + expect(user.last_name).to be_nil + expect(user.first_name(legal: true)).to be_nil + expect(user.last_name(legal: true)).to be_nil end - context "when name entered with comma" do - it "returns the parsed first name" do - user = create(:user, full_name: "Turing, Alan M.") + context "with a preferred name" do + let(:user) { create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi Hacks") } + + it "favor the preferred name" do + expect(user.first_name).to eql("FiFi") + expect(user.last_name).to eql("Hacks") + end + + it "return the legal name when asked" do + expect(user.first_name(legal: true)).to eql("Fiona") + expect(user.last_name(legal: true)).to eql("Hackworth") + end + + it "treat a single-word preferred name as a first name" do + user.update!(preferred_name: "FiFi") - expect(user.first_name).to eql("Alan M.") + expect(user.first_name).to eql("FiFi") + expect(user.last_name).to be_nil + expect(user.last_name(legal: true)).to eql("Hackworth") end end end - describe "#last_name" do - it "returns the stored last name with suffix" do - user = create(:user, full_name: "Ken Griffey Jr.") + describe "#initial_name" do + it "returns the first name and last initial" do + user = create(:user, first_name: "First", last_name: "Last") - expect(user.last_name).to eql("Griffey Jr.") + expect(user.initial_name).to eql("First L") end - context "when name is downcased" do - it "returns the stored last_name" do - user = create(:user, full_name: "ann marie") + it "uses only the first word of a multi-word first name" do + user = create(:user, first_name: "Martin Luther", last_name: "King Jr.") - expect(user.last_name).to eql("marie") - end + expect(user.initial_name).to eql("Martin K") end - context "when entered with comma" do - it "returns the parsed last name" do - user = create(:user, full_name: "Carreño Quiñones, María-Jose") + it "uses the preferred name" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi Hacks") - expect(user.last_name).to eql("Carreño Quiñones") - end + expect(user.initial_name).to eql("FiFi H") end - end - describe "#initial_name" do - it "returns" do - user = create(:user, full_name: "First Last") + it "returns a single-word preferred name as is" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi") - expect(user.initial_name).to eql("First L") + expect(user.initial_name).to eql("FiFi") end - context "when full_name is nil" do - it "returns" do + it "returns a lone first name as is" do + user = create(:user, first_name: "Madonna", last_name: nil) + + expect(user.initial_name).to eql("Madonna") + end + + context "when the user has no name" do + it "returns the email handle" do user = create(:user, email: "user1@example.com", first_name: nil, last_name: nil) expect(user.initial_name).to eql("user1") @@ -193,6 +376,18 @@ end end + describe "onboarding" do + it "ends once a first name is saved" do + user = create(:user, first_name: nil, last_name: nil) + expect(user).to be_onboarding + + user.update!(first_name: "Fiona") + + expect(user).not_to be_onboarding + expect(user).to be_was_onboarding + end + end + describe "teenager status" do let(:first_student_attrs) do { name: "first", league: "frc", team_number: "1234", role: "student_leader" } @@ -274,46 +469,6 @@ end end - describe "#private" do - describe "#namae" do - context "when brackets in name" do - it "can't parse the name" do - user = build(:user, full_name: "Zach Latta [Dev]") - - expect(user).not_to be_valid - expect(user.errors[:full_name]).not_to be_empty - end - end - - context "when parentheses" do - it "can't parse the name" do - user = build(:user, full_name: "Max (test) Wofford") - - expect(user).not_to be_valid - expect(user.errors[:full_name]).not_to be_empty - end - end - - context "when emojis in name" do - it "can't parse the name" do - user = build(:user, full_name: "Melody ✨") - - expect(user).not_to be_valid - expect(user.errors[:full_name]).not_to be_empty - end - end - - context "when a number" do - it "can't parse the name" do - user = build(:user, full_name: "5512700050241863") - - expect(user).not_to be_valid - expect(user.errors[:full_name]).not_to be_empty - end - end - end - end - describe "#pretend_is_not_admin" do it "is cleared on save for users without an admin role" do user = create(:user) diff --git a/spec/requests/users/first_controller_spec.rb b/spec/requests/users/first_controller_spec.rb index 35684b2945..fb80155e5f 100644 --- a/spec/requests/users/first_controller_spec.rb +++ b/spec/requests/users/first_controller_spec.rb @@ -7,7 +7,8 @@ { user: { email: "fresh-#{SecureRandom.hex(4)}@example.invalid", - full_name: "Probe Probe", + first_name: "Probe", + last_name: "Probe", affiliations_attributes: { "0" => { name: "first", @@ -21,6 +22,17 @@ } end + describe "GET /first/new" do + it "asks for a first and last name" do + get "/first/welcome" + + expect(response).to have_http_status(:ok) + expect(response.body).to include('name="user[first_name]"') + expect(response.body).to include('name="user[last_name]"') + expect(response.body).not_to include('name="user[full_name]"') + end + end + describe "POST /first" do it "responds with a redirect when the supplied email already belongs to an existing user" do existing = create(:user, verified: true) @@ -63,7 +75,8 @@ { user: { email: user.email, - full_name: user.full_name, + first_name: user.first_name, + last_name: user.last_name, affiliations_attributes: { "0" => { name: "first", diff --git a/spec/services/user_service/parse_name_spec.rb b/spec/services/user_service/parse_name_spec.rb index c114b2ec13..08699d696b 100644 --- a/spec/services/user_service/parse_name_spec.rb +++ b/spec/services/user_service/parse_name_spec.rb @@ -3,125 +3,77 @@ require "rails_helper" RSpec.describe UserService::ParseName do - describe "#run" do - it "returns self for chaining" do - service = described_class.new(full_name: "John Doe").run - expect(service).to be_a(described_class) - end - - it "handles blank full_name" do - service = described_class.new(full_name: "").run - expect(service.first_name).to be_nil - expect(service.last_name).to be_nil - end - - it "handles nil full_name" do - service = described_class.new(full_name: nil).run - expect(service.first_name).to be_nil - expect(service.last_name).to be_nil - end + def parse(full_name) + result = described_class.new(full_name:).run + [result.first_name, result.last_name] end - describe "#first_name" do - it "parses simple names" do - service = described_class.new(full_name: "John Doe").run - expect(service.first_name).to eq("John") - end - - it "keeps middle names in first_name" do - service = described_class.new(full_name: "Martin Luther King").run - expect(service.first_name).to eq("Martin Luther") - end - - it "handles names with particles (von, de, etc.)" do - service = described_class.new(full_name: "Wernher von Braun").run - expect(service.first_name).to eq("Wernher von") - end - - it "uses family name as fallback when given name is blank" do - # Edge case where Namae only extracts family - service = described_class.new(full_name: "Madonna").run - expect(service.first_name).to eq("Madonna") - end + it "splits a simple name" do + expect(parse("John Doe")).to eq(["John", "Doe"]) end - describe "#last_name" do - it "parses simple names" do - service = described_class.new(full_name: "John Doe").run - expect(service.last_name).to eq("Doe") - end - - it "includes suffixes in last_name" do - service = described_class.new(full_name: "Martin Luther King Jr.").run - expect(service.last_name).to eq("King Jr.") - end - - it "handles multiple word last names" do - service = described_class.new(full_name: "José María García García").run - # Depending on Namae parsing, family might be "García García" or just "García" - expect(service.last_name).not_to be_nil - end + it "keeps middle names with the first name" do + expect(parse("Donald Ervin Knuth")).to eq(["Donald Ervin", "Knuth"]) end - describe "international names" do - it "handles accented characters" do - service = described_class.new(full_name: "José María Martínez").run - expect(service.first_name).not_to be_nil - expect(service.last_name).not_to be_nil - end + it "keeps particles with the first name" do + expect(parse("Wernher von Braun")).to eq(["Wernher von", "Braun"]) + expect(parse("Jean-Claude Van Damme")).to eq(["Jean-Claude Van", "Damme"]) + end - it "preserves unicode characters in parsing" do - service = described_class.new(full_name: "François Müller").run - expect(service.first_name).not_to be_nil - expect(service.last_name).not_to be_nil - end + it "keeps suffixes with the last name" do + expect(parse("Martin Luther King Jr.")).to eq(["Martin Luther", "King Jr."]) + end - it "handles hyphenated first and last names" do - service = described_class.new(full_name: "Jean-Claude Van Damme").run - expect(service.first_name).not_to be_nil - expect(service.last_name).not_to be_nil - end + it "understands the comma form" do + expect(parse("Doe, John")).to eq(["John", "Doe"]) + expect(parse("Turing, Alan M.")).to eq(["Alan M.", "Turing"]) + expect(parse("Carreño Quiñones, María-Jose")).to eq(["María-Jose", "Carreño Quiñones"]) end - describe "composite names" do - it "doesn't fail on single names" do - service = described_class.new(full_name: "Madonna").run - expect(service.first_name).to eq("Madonna") - expect(service.last_name).to be_nil - end + it "drops honorific titles" do + expect(parse("Prof. Donald Ervin Knuth")).to eq(["Donald Ervin", "Knuth"]) + expect(parse("Sir Elton John")).to eq(["Elton", "John"]) + end - it "handles comma-separated names" do - service = described_class.new(full_name: "Doe, John").run - # Namae can parse this format - expect(service.first_name).not_to be_nil - expect(service.last_name).not_to be_nil - end + it "puts a single name in first_name" do + expect(parse("Madonna")).to eq(["Madonna", nil]) + expect(parse("O'Brien")).to eq(["O'Brien", nil]) + end - it "handles titles in names" do - service = described_class.new(full_name: "Prof. Donald Ervin Knuth").run - # Namae strips titles - should work fine - expect(service.first_name).not_to be_nil - expect(service.last_name).not_to be_nil - end + it "handles lowercase names" do + expect(parse("ann marie")).to eq(["ann", "marie"]) end - describe "component accessors" do - let(:parsed) { described_class.new(full_name: "Martin Luther King Jr.").run } + it "normalizes whitespace" do + expect(parse(" John Doe ")).to eq(["John", "Doe"]) + end - it "exposes given_name" do - expect(parsed.given_name).to eq("Martin Luther") - end + it "returns nils for a blank name" do + expect(parse(nil)).to eq([nil, nil]) + expect(parse("")).to eq([nil, nil]) + expect(parse(" ")).to eq([nil, nil]) + end - it "exposes family_name" do - expect(parsed.family_name).to eq("King") + # Namae has a list of honorifics (Elder, Lord, Pastor, Dr, ...) and gives up + # or misparses when a surname collides with one. See hackclub/hcb#15467. + describe "surnames that collide with Namae's title list" do + it "falls back to a positional split when Namae returns nothing" do + expect(parse("Jane Elder")).to eq(["Jane", "Elder"]) + expect(parse("Mary Lord")).to eq(["Mary", "Lord"]) + expect(parse("Peter Pastor")).to eq(["Peter", "Pastor"]) end - it "exposes particle" do - expect(parsed.particle).to be_nil + it "does not treat a trailing surname as a title" do + expect(parse("Jane W Elder")).to eq(["Jane W", "Elder"]) + expect(parse("Ann Marie Count")).to eq(["Ann Marie", "Count"]) end - it "exposes suffix" do - expect(parsed.suffix).to eq("Jr.") + it "covers every word in the title list" do + %w[Sir Lord Count Countess Gen Adm Col Maj Capt Cmdr Lt Sgt Cpl Pvt Pastor Pr Reverend Rev Elder Deacon + Deaconess Father Fr Rabbi Cantor Vicar Prof Dr Md PhD].each do |surname| + expect(parse("Jane #{surname}")).to eq(["Jane", surname]), "expected \"Jane #{surname}\" to split positionally" + end end end end From 905ca53c7784a9ee4e36ebaf7beee8216b79aea8 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Mon, 2 Nov 2026 14:30:48 -0800 Subject: [PATCH 20/23] Update specs that still relied on full_name as the user name input Co-Authored-By: Claude Mythos 5.1 --- spec/controllers/admin_controller_spec.rb | 2 +- spec/requests/paper_trail_impersonation_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/controllers/admin_controller_spec.rb b/spec/controllers/admin_controller_spec.rb index 91e07a9896..7f58ddbb61 100644 --- a/spec/controllers/admin_controller_spec.rb +++ b/spec/controllers/admin_controller_spec.rb @@ -191,7 +191,7 @@ it "labels nameless and space-padded users with trimmed text" do admin = create(:user, :make_admin) create_session(admin, verified: true) - nameless = create(:user, full_name: nil) + nameless = create(:user, first_name: nil, last_name: nil) padded = create(:user, full_name: "Jane Doe ") get(:user_search, format: :json) diff --git a/spec/requests/paper_trail_impersonation_spec.rb b/spec/requests/paper_trail_impersonation_spec.rb index b8329b2bd4..d4e2e8db94 100644 --- a/spec/requests/paper_trail_impersonation_spec.rb +++ b/spec/requests/paper_trail_impersonation_spec.rb @@ -15,7 +15,7 @@ def sign_in_with(session) it "records the impersonator on versions created while impersonating" do sign_in_with(create(:user_session, user:, impersonated_by: admin, expiration_at: 1.hour.from_now)) - patch "/users/#{user.id}", params: { user: { full_name: "Changed Name" } } + patch "/users/#{user.id}", params: { user: { first_name: "Changed", last_name: "Name" } } version = user.versions.last expect(version.whodunnit).to eq(user.id.to_s) @@ -25,7 +25,7 @@ def sign_in_with(session) it "leaves the impersonator blank on versions created without impersonation" do sign_in_with(create(:user_session, user:, expiration_at: 1.hour.from_now)) - patch "/users/#{user.id}", params: { user: { full_name: "Changed Name" } } + patch "/users/#{user.id}", params: { user: { first_name: "Changed", last_name: "Name" } } version = user.versions.last expect(version.whodunnit).to eq(user.id.to_s) From 9f2b9cb3907e06c6814cf7c47dea79822dfac26f Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Mon, 2 Nov 2026 14:49:12 -0800 Subject: [PATCH 21/23] Use add_column instead of change_table in first/last name migration strong_migrations can't inspect change_table blocks, so re-applying the migration failed the Reversible Migrations check. Co-Authored-By: Claude Opus 5.5 --- .../20261102000001_add_first_and_last_names_to_users.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/db/migrate/20261102000001_add_first_and_last_names_to_users.rb b/db/migrate/20261102000001_add_first_and_last_names_to_users.rb index 54adf6edac..db40c28113 100644 --- a/db/migrate/20261102000001_add_first_and_last_names_to_users.rb +++ b/db/migrate/20261102000001_add_first_and_last_names_to_users.rb @@ -2,10 +2,8 @@ class AddFirstAndLastNamesToUsers < ActiveRecord::Migration[8.1] def change - change_table :users, bulk: true do |t| - t.string :first_name - t.string :last_name - end + add_column :users, :first_name, :string + add_column :users, :last_name, :string end end From fdfe6319996821ec7b1ace33946998c0e810f5f9 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Mon, 9 Nov 2026 22:33:44 -0800 Subject: [PATCH 22/23] fix name corruption --- app/models/user.rb | 37 ++++++++--- app/services/user_service/parse_name.rb | 20 +++--- app/views/api/v4/users/_user.json.jbuilder | 6 +- app/views/users/first/new.html.erb | 4 +- .../api/v4/card_grants_controller_spec.rb | 4 +- .../api/v4/events_controller_spec.rb | 23 ++++++- spec/controllers/users_controller_spec.rb | 11 ++++ spec/models/user_spec.rb | 61 ++++++++++++++++--- spec/services/user_service/parse_name_spec.rb | 10 ++- 9 files changed, 139 insertions(+), 37 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 7ec5d2264b..fba63d454c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -235,6 +235,9 @@ class User < ApplicationRecord # so a first name is only required once there is a last name to go with it # or once a name has been set (it can be changed, but not removed). validates :first_name, presence: true, if: -> { self[:last_name].present? || clearing_existing_name? } + # Stripe Issuing needs a last name, and cardholder names are synced to Stripe + # on every profile update. + validates :last_name, presence: { message: "is required for HCB cards" }, if: -> { last_name_changed? && stripe_cardholder.present? } validates :first_name, format: { with: NAME_CHARACTERS_REGEX, message: NAME_CHARACTERS_MESSAGE, allow_blank: true } validates :last_name, format: { with: NAME_CHARACTERS_REGEX, message: NAME_CHARACTERS_MESSAGE, allow_blank: true } @@ -353,25 +356,29 @@ def make_admin! admin! end - # The user's first name as stored, which may include middle names and - # particles ("Donald Ervin", "Wernher von"). + # With `legal: true`, the first name as stored, which may include middle + # names ("Donald Ervin"). Use it for anything that must match the user's + # legal name (Stripe, forms). # - # Without `legal: true` a preferred name takes precedence, so greetings and - # other display uses call people what they asked to be called. Pass - # `legal: true` for anything that must match the user's legal name (Stripe). + # Otherwise, the first word of what we call the user in greetings and other + # display uses ("Hey Donald,"): their preferred name if set, then their legal + # first name, then their email handle if they haven't added a name yet. def first_name(legal: false) - return preferred_name_parts.first if !legal && preferred_name.present? + stored = name_split? ? self[:first_name] : legacy_parsed_name.first_name + return stored if legal - self[:first_name] || legacy_parsed_name.first_name + display_name = preferred_name.presence || stored.presence || (email_handle if email.present?) + display_name&.split(" ")&.first end - # The user's last name as stored, which may include a suffix ("King Jr."). + # The user's last name as stored, which may include particles and a suffix + # ("de la Cruz", "King Jr."). # Returns nil for users who only have a first name, and for single-word # preferred names when `legal` is false. def last_name(legal: false) return preferred_name_parts.last if !legal && preferred_name.present? - self[:last_name] || legacy_parsed_name.last_name + name_split? ? self[:last_name] : legacy_parsed_name.last_name end # "Fiona H" — what we show to other users. @@ -380,7 +387,8 @@ def initial_name last = last_name return name if first.blank? || last.blank? - "#{first.split(" ").first[0..20]} #{last[0]}" + # Initial of the surname itself, skipping lowercase particles ("de la Cruz" -> "C"). + "#{first[0..20]} #{last[/[[:upper:]]/] || last[0]}" end def safe_name @@ -810,6 +818,15 @@ def preferred_name_parts [words[0..-2].join(" "), words.last] end + # Whether `first_name` / `last_name` hold this user's name. Only rows that + # predate the split columns and haven't been backfilled (or edited) yet fall + # back to parsing `full_name`. A blank `last_name` on a split row means the + # user has a single name, and must not be re-derived from `full_name` + # ("Mary Ann" would otherwise come back as last name "Ann"). + def name_split? + first_name_in_database.present? || first_name_changed? || last_name_changed? + end + # Rows written before the split columns existed and not yet backfilled. def legacy_parsed_name UserService::ParseName.new(full_name:).run diff --git a/app/services/user_service/parse_name.rb b/app/services/user_service/parse_name.rb index 0c9e5dca7b..f704dd2a27 100644 --- a/app/services/user_service/parse_name.rb +++ b/app/services/user_service/parse_name.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "namae" - module UserService # Splits a single-field full name into the first and last name stored on # `users.first_name` / `users.last_name`. @@ -13,8 +11,9 @@ module UserService # Namae handles comma forms ("Doe, John"), particles ("Wernher von Braun") # and suffixes ("King Jr."). It also keeps a list of honorifics, and a # surname that collides with one ("Jane Elder", "Mary Lord") makes it either - # give up entirely or treat the surname as a title. In those cases we fall - # back to a positional split so a real surname is never lost. + # give up entirely or treat the surname as a title. In those cases, and for + # all-lowercase names it can't read, we fall back to a positional split so a + # real surname is never lost. class ParseName Result = Struct.new(:first_name, :last_name, keyword_init: true) @@ -26,10 +25,11 @@ def run return Result.new(first_name: nil, last_name: nil) if @full_name.blank? parsed = Namae.parse(@full_name).first - return positional_split if parsed.nil? || trailing_word_taken_as_title?(parsed) + return positional_split if parsed.nil? || trailing_word_taken_as_title?(parsed) || given_name_taken_as_particle?(parsed) - first = [parsed.given, parsed.particle].compact_blank.join(" ").presence - last = [parsed.family, parsed.suffix].compact_blank.join(" ").presence + # Particles belong to the surname: "Juan de la Cruz" is "Juan" + "de la Cruz". + first = parsed.given.presence + last = [parsed.particle, parsed.family, parsed.suffix].compact_blank.join(" ").presence # "Madonna" parses as given-only; "Mr. Smith" parses as family-only. # Either way the one word we have belongs in first_name. @@ -46,6 +46,12 @@ def trailing_word_taken_as_title?(parsed) parsed.title.present? && @full_name.downcase.end_with?(parsed.title.downcase) end + # Namae reads leading lowercase words as particles, so "john smith" comes + # back with no given name and "john" as a particle. + def given_name_taken_as_particle?(parsed) + parsed.given.blank? && parsed.particle.present? + end + def positional_split words = @full_name.split(" ") return Result.new(first_name: words.first, last_name: nil) if words.one? diff --git a/app/views/api/v4/users/_user.json.jbuilder b/app/views/api/v4/users/_user.json.jbuilder index 8649584d5c..78676c099d 100644 --- a/app/views/api/v4/users/_user.json.jbuilder +++ b/app/views/api/v4/users/_user.json.jbuilder @@ -12,11 +12,13 @@ object_shape(json, user, created_at: false) do # those which we'd like to expose less of: expand_pii(override_if: user == current_user || local_assigns[:show_email]) do json.email user.email - json.first_name user.first_name(legal: true) - json.last_name user.last_name(legal: true) end + # Legal names stay with the user (or admins with the `pii` scope): org + # members see `name`, which respects the preferred name. expand_pii(override_if: user == current_user) do + json.first_name user.first_name(legal: true) + json.last_name user.last_name(legal: true) json.birthday user.birthday if expand?(:shipping_address) json.shipping_address do diff --git a/app/views/users/first/new.html.erb b/app/views/users/first/new.html.erb index 5a80154f52..77389baaf2 100644 --- a/app/views/users/first/new.html.erb +++ b/app/views/users/first/new.html.erb @@ -37,11 +37,11 @@
<%= form.label :first_name, "First name" %> - <%= form.text_field :first_name, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %> + <%= form.text_field :first_name, value: @user.first_name(legal: true), placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %>
<%= form.label :last_name, "Last name" %> - <%= form.text_field :last_name, placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %> + <%= form.text_field :last_name, value: @user.last_name(legal: true), placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %>
diff --git a/spec/controllers/api/v4/card_grants_controller_spec.rb b/spec/controllers/api/v4/card_grants_controller_spec.rb index 24ed3e4dd3..9b50639627 100644 --- a/spec/controllers/api/v4/card_grants_controller_spec.rb +++ b/spec/controllers/api/v4/card_grants_controller_spec.rb @@ -94,8 +94,8 @@ def card_grant_params "object" => "user", "name" => "Orpheus D", "email" => "orpheus@hackclub.com", - "first_name" => "Orpheus the", - "last_name" => "Dinosaur", + "first_name" => "Orpheus", + "last_name" => "the Dinosaur", "admin" => false, "auditor" => false, "avatar" => "https://gravatar.com/avatar/stubbed", diff --git a/spec/controllers/api/v4/events_controller_spec.rb b/spec/controllers/api/v4/events_controller_spec.rb index 14a7f9e490..98c5719090 100644 --- a/spec/controllers/api/v4/events_controller_spec.rb +++ b/spec/controllers/api/v4/events_controller_spec.rb @@ -27,8 +27,25 @@ expect(other_user_data).to include("email" => "other@example.com") end - it "exposes the current user's own email" do - current_user = create(:user, email: "me@example.com") + it "does not expose another organizer's legal name over their preferred name" do + current_user = create(:user) + other_user = create(:user, first_name: "Zebulon", last_name: "Quixley", preferred_name: "Zeb Q") + event = create(:event) + create(:organizer_position, user: current_user, event:) + create(:organizer_position, user: other_user, event:) + + token = create(:api_token, user: current_user) + request.headers["Authorization"] = "Bearer #{token.token}" + + get :show, params: { id: event.friendly_id, expand: "users" }, as: :json + + other_user_data = response.parsed_body["users"].find { |u| u["id"] == other_user.public_id } + expect(other_user_data).not_to include("first_name", "last_name") + expect(response.body).not_to include("Zebulon", "Quixley") + end + + it "exposes the current user's own email and legal name" do + current_user = create(:user, email: "me@example.com", first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi") event = create(:event) create(:organizer_position, user: current_user, event:) @@ -39,7 +56,7 @@ expect(response).to have_http_status(:ok) me = response.parsed_body["users"].find { |u| u["id"] == current_user.public_id } - expect(me).to include("email" => "me@example.com") + expect(me).to include("email" => "me@example.com", "first_name" => "Fiona", "last_name" => "Hackworth") end it "exposes email of another organizer on the same sub event" do diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index c0a53cc2bd..9409334435 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -269,6 +269,17 @@ expect(response.body).to include('name="user[first_name]"').and include('value="Fiona"') expect(response.body).to include('name="user[last_name]"').and include('value="Hackworth"') end + + it "leaves the last name blank for a user with a multi-word first name and no last name" do + user = create(:user, first_name: "Mary Ann", last_name: nil, phone_number: "+18556254225") + create_session(user, verified: true) + + get(:edit, params: { id: user.id }) + + page = Nokogiri::HTML(response.body) + expect(page.at_css("input[name='user[first_name]']")["value"]).to eq("Mary Ann") + expect(page.at_css("input[name='user[last_name]']")["value"]).to be_blank + end end describe "#update" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7fc5e417e9..799659127b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -112,11 +112,38 @@ expect(user.last_name).to be_nil end + it "does not derive a last name from a multi-word first name" do + user = create(:user, first_name: "Mary Ann", last_name: nil).reload + + expect(user.first_name(legal: true)).to eq("Mary Ann") + expect(user.last_name(legal: true)).to be_nil + expect(user.last_name).to be_nil + expect(user.initial_name).to eq("Mary Ann") + end + + it "initials the surname, not its lowercase particle" do + user = create(:user, first_name: "Juan", last_name: "de la Cruz") + + expect(user.initial_name).to eq("Juan C") + expect(user.first_name).to eq("Juan") + end + + it "does not refill a first name that was just cleared" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + + expect(user.update(first_name: "")).to be(false) + expect(user.first_name(legal: true)).to be_nil + end + + it "returns a nil display first name for a blank new user instead of raising" do + expect(User.new.first_name).to be_nil + end + it "splits a full_name assigned by a legacy writer" do user = create(:user, full_name: "Martin Luther King Jr.") - expect(user.first_name).to eq("Martin Luther") - expect(user.last_name).to eq("King Jr.") + expect(user.first_name(legal: true)).to eq("Martin Luther") + expect(user.last_name(legal: true)).to eq("King Jr.") expect(user.full_name).to eq("Martin Luther King Jr.") end @@ -145,7 +172,9 @@ it "trims and collapses whitespace" do user = create(:user, first_name: " Fiona Rose ", last_name: " Hackworth ") - expect(user).to have_attributes(first_name: "Fiona Rose", last_name: "Hackworth", full_name: "Fiona Rose Hackworth") + expect(user.first_name(legal: true)).to eq("Fiona Rose") + expect(user.last_name(legal: true)).to eq("Hackworth") + expect(user.full_name).to eq("Fiona Rose Hackworth") end it "treats a blank last name as no last name" do @@ -233,6 +262,22 @@ expect(user.reload.full_name).to eq("Fiona") end + it "does not allow a cardholder to remove their last name" do + user = create(:user, first_name: "Fiona", last_name: "Hackworth") + create(:stripe_cardholder, user:) + + expect(user.update(last_name: "")).to be(false) + expect(user.errors[:last_name]).to include("is required for HCB cards") + expect(user.reload.full_name).to eq("Fiona Hackworth") + end + + it "lets a cardholder with no last name on file update other details" do + user = create(:user, first_name: "Madonna", last_name: nil) + create(:stripe_cardholder, user:) + + expect(user.reload.update(first_name: "Madonna Louise")).to be(true) + end + it "accepts accented letters and name punctuation" do user = build(:user, first_name: "María-José", last_name: "O'Brien, Jr.") @@ -295,19 +340,19 @@ end describe "#first_name and #last_name" do - it "return the stored names" do + it "return the stored names, with only the first word for display" do user = create(:user, first_name: "Donald Ervin", last_name: "Knuth") - expect(user.first_name).to eql("Donald Ervin") + expect(user.first_name).to eql("Donald") expect(user.last_name).to eql("Knuth") expect(user.first_name(legal: true)).to eql("Donald Ervin") expect(user.last_name(legal: true)).to eql("Knuth") end - it "return nil for a user without a name" do - user = create(:user, first_name: nil, last_name: nil) + it "return nil for a user without a name, except the display first name" do + user = create(:user, email: "fiona.h@example.com", first_name: nil, last_name: nil) - expect(user.first_name).to be_nil + expect(user.first_name).to eq("fiona.h") expect(user.last_name).to be_nil expect(user.first_name(legal: true)).to be_nil expect(user.last_name(legal: true)).to be_nil diff --git a/spec/services/user_service/parse_name_spec.rb b/spec/services/user_service/parse_name_spec.rb index 08699d696b..4bef34df57 100644 --- a/spec/services/user_service/parse_name_spec.rb +++ b/spec/services/user_service/parse_name_spec.rb @@ -16,9 +16,10 @@ def parse(full_name) expect(parse("Donald Ervin Knuth")).to eq(["Donald Ervin", "Knuth"]) end - it "keeps particles with the first name" do - expect(parse("Wernher von Braun")).to eq(["Wernher von", "Braun"]) - expect(parse("Jean-Claude Van Damme")).to eq(["Jean-Claude Van", "Damme"]) + it "keeps particles with the last name" do + expect(parse("Wernher von Braun")).to eq(["Wernher", "von Braun"]) + expect(parse("Juan de la Cruz")).to eq(["Juan", "de la Cruz"]) + expect(parse("Jean-Claude Van Damme")).to eq(["Jean-Claude", "Van Damme"]) end it "keeps suffixes with the last name" do @@ -41,8 +42,11 @@ def parse(full_name) expect(parse("O'Brien")).to eq(["O'Brien", nil]) end + # Namae reads leading lowercase words as particles and finds no given name. it "handles lowercase names" do expect(parse("ann marie")).to eq(["ann", "marie"]) + expect(parse("fiona rose hackworth")).to eq(["fiona rose", "hackworth"]) + expect(parse("john Smith")).to eq(["john", "Smith"]) end it "normalizes whitespace" do From 37847f11457574abff5fa43cc32727454e139e66 Mon Sep 17 00:00:00 2001 From: Manu Gurudath Date: Tue, 10 Nov 2026 15:22:22 -0800 Subject: [PATCH 23/23] fix bugs with name separation --- .../event/applications_controller.rb | 2 +- app/controllers/users/first_controller.rb | 2 +- app/controllers/users_controller.rb | 4 +- app/models/stripe_cardholder.rb | 4 +- app/models/user.rb | 113 ++++++------ app/services/user_service/parse_name.rb | 37 ++-- .../backfill_user_legal_names_task.rb | 18 ++ app/views/api/v4/users/_user.json.jbuilder | 3 +- app/views/event/applications/edit.html.erb | 8 +- .../event/applications/personal_info.html.erb | 8 +- app/views/users/edit.html.erb | 24 +-- app/views/users/first/new.html.erb | 8 +- config/locales/en.yml | 2 - ...00001_add_first_and_last_names_to_users.rb | 9 - ...add_legal_first_and_last_names_to_users.rb | 9 + ...0002_backfill_user_first_and_last_names.rb | 39 ---- db/schema.rb | 6 +- spec/controllers/admin_controller_spec.rb | 2 +- .../api/v4/events_controller_spec.rb | 22 ++- spec/controllers/logins_controller_spec.rb | 2 +- spec/controllers/users_controller_spec.rb | 73 +++++--- spec/factories/user_factory.rb | 7 +- spec/misc/flipper_spec.rb | 4 +- spec/models/stripe_cardholder_spec.rb | 14 +- spec/models/user_spec.rb | 166 ++++++++++++------ .../event/applications_controller_spec.rb | 75 ++++++++ .../paper_trail_impersonation_spec.rb | 4 +- spec/requests/users/first_controller_spec.rb | 12 +- spec/services/user_service/parse_name_spec.rb | 12 ++ .../backfill_user_legal_names_task_spec.rb | 54 ++++++ 30 files changed, 470 insertions(+), 273 deletions(-) create mode 100644 app/tasks/maintenance/backfill_user_legal_names_task.rb delete mode 100644 db/migrate/20261102000001_add_first_and_last_names_to_users.rb create mode 100644 db/migrate/20261102000001_add_legal_first_and_last_names_to_users.rb delete mode 100644 db/migrate/20261102000002_backfill_user_first_and_last_names.rb create mode 100644 spec/requests/event/applications_controller_spec.rb create mode 100644 spec/tasks/maintenance/backfill_user_legal_names_task_spec.rb diff --git a/app/controllers/event/applications_controller.rb b/app/controllers/event/applications_controller.rb index 239bbb01dc..90b01fbe34 100644 --- a/app/controllers/event/applications_controller.rb +++ b/app/controllers/event/applications_controller.rb @@ -323,7 +323,7 @@ def application_params end def user_params - params.require(:event_application).permit(:first_name, :last_name, :preferred_name, :phone_number, :birthday) + params.require(:event_application).permit(:legal_first_name, :legal_last_name, :preferred_name, :phone_number, :birthday) end def record_pageview diff --git a/app/controllers/users/first_controller.rb b/app/controllers/users/first_controller.rb index 3758c2d569..2e65c14fba 100644 --- a/app/controllers/users/first_controller.rb +++ b/app/controllers/users/first_controller.rb @@ -161,7 +161,7 @@ def create private def user_params - params.require(:user).permit(:email, :first_name, :last_name, affiliations_attributes: [:league, :team_number, :name, :team_name, :role]) + params.require(:user).permit(:email, :legal_first_name, :legal_last_name, affiliations_attributes: [:league, :team_number, :name, :team_name, :role]) end def user_ref_params diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 24cf633e6a..88a4cc9fde 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -576,8 +576,8 @@ def set_onboarding def user_params attributes = [ # account - :first_name, - :last_name, + :legal_first_name, + :legal_last_name, :preferred_name, :phone_number, :birthday, diff --git a/app/models/stripe_cardholder.rb b/app/models/stripe_cardholder.rb index 4cb265cb19..ac59f60923 100644 --- a/app/models/stripe_cardholder.rb +++ b/app/models/stripe_cardholder.rb @@ -136,9 +136,7 @@ def self.last_name(user) end def self.clean_name(name, field: "name") - # Stripe Issuing needs both names, but HCB lets people go by a first name - # alone. Say so plainly instead of letting the character check below - # blame "special characters" for a name that was never entered. + # HCB lets people go by a first name alone, but Stripe Issuing needs both names. if name.blank? raise Errors::StripeInvalidNameError, "A legal #{field} is required to issue cards. Please add it in your account settings." end diff --git a/app/models/user.rb b/app/models/user.rb index fba63d454c..7d23cafc84 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -13,10 +13,10 @@ # comment_notifications :integer default(0), not null # creation_method :integer # email :text not null -# first_name :string # full_name :string # joined_as_teenager :boolean -# last_name :string +# legal_first_name :string +# legal_last_name :string # locked_at :datetime # monthly_donation_summary :boolean default(TRUE) # monthly_follower_summary :boolean default(TRUE) @@ -50,8 +50,7 @@ # index_users_on_slug (slug) UNIQUE # class User < ApplicationRecord - # Latin letters (including common accented ones) plus the punctuation that - # legitimately appears in names. Stripe Issuing only accepts latin names. + # Stripe Issuing only accepts latin names. NAME_CHARACTERS_REGEX = /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,' -]+\z/ NAME_CHARACTERS_MESSAGE = "may only contain letters, spaces, hyphens, apostrophes, periods, and commas." @@ -206,11 +205,10 @@ class User < ApplicationRecord before_save :clear_pretend_is_not_admin, if: -> { pretend_is_not_admin? && !admin_override_pretend? } before_create :format_number - # `first_name` / `last_name` are the source of truth and `full_name` is kept - # in sync from them. Legacy writers that assign `full_name` directly (older - # clients, scripts) have it split into the two columns instead. - before_validation :split_full_name, if: -> { full_name_changed? && !first_name_changed? && !last_name_changed? } - before_validation :sync_full_name, if: -> { first_name_changed? || last_name_changed? } + # The legal name columns are the source of truth; `full_name` is kept in sync with them. + before_validation :fill_legacy_legal_name, if: -> { awaiting_legal_name_backfill? && legal_name_parts_changed? && !full_name_changed? } + before_validation :split_full_name, if: -> { full_name_changed? && !legal_name_parts_changed? } + before_validation :sync_full_name, if: :legal_name_edited? before_save :on_phone_number_update validate :second_factor_present_for_2fa @@ -231,24 +229,13 @@ class User < ApplicationRecord validates_presence_of :birthday, if: -> { birthday_ciphertext_in_database.present? } - # Users are created with just an email and add their name during onboarding, - # so a first name is only required once there is a last name to go with it - # or once a name has been set (it can be changed, but not removed). - validates :first_name, presence: true, if: -> { self[:last_name].present? || clearing_existing_name? } - # Stripe Issuing needs a last name, and cardholder names are synced to Stripe - # on every profile update. - validates :last_name, presence: { message: "is required for HCB cards" }, if: -> { last_name_changed? && stripe_cardholder.present? } - validates :first_name, format: { with: NAME_CHARACTERS_REGEX, message: NAME_CHARACTERS_MESSAGE, allow_blank: true } - validates :last_name, format: { with: NAME_CHARACTERS_REGEX, message: NAME_CHARACTERS_MESSAGE, allow_blank: true } + # Names are added during onboarding, and can then be changed but not removed. + validates :legal_first_name, presence: true, if: -> { legal_last_name.present? || clearing_existing_name? } + # Stripe Issuing requires a last name. + validates :legal_last_name, presence: { message: "is required for HCB cards" }, if: -> { legal_last_name_changed? && stripe_cardholder.present? } + validates :legal_first_name, :legal_last_name, format: { with: NAME_CHARACTERS_REGEX, message: NAME_CHARACTERS_MESSAGE, allow_blank: true } - normalizes :first_name, with: ->(name) { name.squish.presence } - normalizes :last_name, with: ->(name) { name.squish.presence } - - # Validate the stored columns, not the `first_name` / `last_name` readers - # below (which favor the preferred name and fall back to `full_name`). - def read_attribute_for_validation(attribute) - attribute.to_s.in?(%w[first_name last_name]) ? self[attribute] : super - end + normalizes :legal_first_name, :legal_last_name, with: ->(name) { name.squish.presence } validates :email, uniqueness: true, presence: true validates_email_format_of :email @@ -263,7 +250,7 @@ def read_attribute_for_validation(attribute) validates :preferred_name, length: { maximum: 30 } validates :preferred_name, format: { - with: /\A[a-zA-ZàáâäãåąčćęèéêëėįìíîïłńòóôöõøùúûüųūÿýżźñçčšžÀÁÂÄÃÅĄĆČĖĘÈÉÊËÌÍÎÏĮŁŃÒÓÔÖÕØÙÚÛÜŲŪŸÝŻŹÑßÇŒÆČŠŽ∂ð.,' -]+\z/, + with: NAME_CHARACTERS_REGEX, message: "must only contain characters in the latin alphabet.", allow_blank: true, }, if: :preferred_name_changed? @@ -356,36 +343,29 @@ def make_admin! admin! end - # With `legal: true`, the first name as stored, which may include middle - # names ("Donald Ervin"). Use it for anything that must match the user's - # legal name (Stripe, forms). - # - # Otherwise, the first word of what we call the user in greetings and other - # display uses ("Hey Donald,"): their preferred name if set, then their legal - # first name, then their email handle if they haven't added a name yet. + # The legal first name may include middle names ("Donald Ervin"). + # Otherwise, the first word of the name we greet the user by ("Hey Donald,"). def first_name(legal: false) - stored = name_split? ? self[:first_name] : legacy_parsed_name.first_name + stored = legal_name_split? ? legal_first_name : legacy_parsed_name.first_name return stored if legal display_name = preferred_name.presence || stored.presence || (email_handle if email.present?) display_name&.split(" ")&.first end - # The user's last name as stored, which may include particles and a suffix - # ("de la Cruz", "King Jr."). - # Returns nil for users who only have a first name, and for single-word - # preferred names when `legal` is false. + # The legal last name may include particles and a suffix ("de la Cruz", "King Jr."). def last_name(legal: false) return preferred_name_parts.last if !legal && preferred_name.present? - name_split? ? self[:last_name] : legacy_parsed_name.last_name + legal_name_split? ? legal_last_name : legacy_parsed_name.last_name end - # "Fiona H" — what we show to other users. + # "Fiona H", shown to other users. def initial_name first = first_name last = last_name - return name if first.blank? || last.blank? + # Only the first word, so a full name typed into the first name field stays private. + return first || name if last.blank? # Initial of the surname itself, skipping lowercase particles ("de la Cruz" -> "C"). "#{first[0..20]} #{last[/[[:upper:]]/] || last[0]}" @@ -396,7 +376,7 @@ def safe_name return name unless name.length > 24 return full_name if full_name.present? && full_name.length <= 24 - initial_name + initial_name.first(24) end def name @@ -794,23 +774,41 @@ def onboarding_contractor_positions def split_full_name parsed = UserService::ParseName.new(full_name:).run - self.first_name = parsed.first_name - self.last_name = parsed.last_name + self.legal_first_name = parsed.first_name + self.legal_last_name = parsed.last_name end def sync_full_name - self.full_name = [self[:first_name], self[:last_name]].compact_blank.join(" ").presence + self.full_name = [legal_first_name, legal_last_name].compact_blank.join(" ").presence + end + + def legal_name_parts_changed? + legal_first_name_changed? || legal_last_name_changed? + end + + def awaiting_legal_name_backfill? + legal_first_name_in_database.blank? && full_name_in_database.present? + end + + # A blank part keeps its legacy value, so a partial edit can't drop half of the name. + def fill_legacy_legal_name + self.legal_first_name ||= legacy_parsed_name.first_name + self.legal_last_name ||= legacy_parsed_name.last_name + end + + # Saving a legacy name back unchanged only backfills it, leaving `full_name` (and any title) as is. + def legal_name_edited? + return false unless legal_name_parts_changed? + + !awaiting_legal_name_backfill? || [legal_first_name, legal_last_name] != legacy_parsed_name.to_a end - # A name can be changed but not removed once set. Rows that haven't been - # backfilled yet (`full_name` set, `first_name` nil) are only held to this - # when the name itself is being edited, so unrelated updates still save. + # Only checked when the name is edited, so rows awaiting backfill can still save unrelated updates. def clearing_existing_name? - full_name_in_database.present? && (first_name_changed? || last_name_changed? || full_name_changed?) + full_name_in_database.present? && (legal_name_parts_changed? || full_name_changed?) end - # Preferred names are free-form ("FiFi", "FiFi Hacks"), so split them - # positionally: everything but the last word is the first name. + # Preferred names are free-form, so everything but the last word is the first name. def preferred_name_parts words = preferred_name.squish.split(" ") return [words.first, nil] if words.one? @@ -818,16 +816,11 @@ def preferred_name_parts [words[0..-2].join(" "), words.last] end - # Whether `first_name` / `last_name` hold this user's name. Only rows that - # predate the split columns and haven't been backfilled (or edited) yet fall - # back to parsing `full_name`. A blank `last_name` on a split row means the - # user has a single name, and must not be re-derived from `full_name` - # ("Mary Ann" would otherwise come back as last name "Ann"). - def name_split? - first_name_in_database.present? || first_name_changed? || last_name_changed? + # False only for rows awaiting backfill, whose names are still parsed from `full_name`. + def legal_name_split? + legal_first_name_in_database.present? || legal_name_parts_changed? end - # Rows written before the split columns existed and not yet backfilled. def legacy_parsed_name UserService::ParseName.new(full_name:).run end diff --git a/app/services/user_service/parse_name.rb b/app/services/user_service/parse_name.rb index f704dd2a27..e83c4c1da2 100644 --- a/app/services/user_service/parse_name.rb +++ b/app/services/user_service/parse_name.rb @@ -1,38 +1,33 @@ # frozen_string_literal: true module UserService - # Splits a single-field full name into the first and last name stored on - # `users.first_name` / `users.last_name`. - # - # Only used where a full name still arrives as one string: the backfill of - # rows that predate the split columns, and legacy writers that assign - # `User#full_name` directly. Everything else should set the two columns. - # - # Namae handles comma forms ("Doe, John"), particles ("Wernher von Braun") - # and suffixes ("King Jr."). It also keeps a list of honorifics, and a - # surname that collides with one ("Jane Elder", "Mary Lord") makes it either - # give up entirely or treat the surname as a title. In those cases, and for - # all-lowercase names it can't read, we fall back to a positional split so a - # real surname is never lost. + # Splits a legacy single-field full name into a first and last name. + # Namae handles commas, particles and suffixes; anything it can't read is split positionally. class ParseName Result = Struct.new(:first_name, :last_name, keyword_init: true) + # Credentials after a full name ("Jane Smith, PhD") aren't part of the legal name. + TRAILING_CREDENTIAL = /\A(\S+ .+?),? (?:m\.?d|ph\.?d)\.?\z/i + def initialize(full_name:) - @full_name = full_name.to_s.squish + @full_name = full_name.to_s.squish.sub(TRAILING_CREDENTIAL, '\1') end def run return Result.new(first_name: nil, last_name: nil) if @full_name.blank? - parsed = Namae.parse(@full_name).first - return positional_split if parsed.nil? || trailing_word_taken_as_title?(parsed) || given_name_taken_as_particle?(parsed) + names = Namae.parse(@full_name) + # Namae returns nothing for surnames on its title list, and several names for "John and Jane Doe". + return positional_split unless names.one? + + parsed = names.first + return positional_split if trailing_word_taken_as_title?(parsed) || given_name_taken_as_particle?(parsed) # Particles belong to the surname: "Juan de la Cruz" is "Juan" + "de la Cruz". first = parsed.given.presence last = [parsed.particle, parsed.family, parsed.suffix].compact_blank.join(" ").presence - # "Madonna" parses as given-only; "Mr. Smith" parses as family-only. - # Either way the one word we have belongs in first_name. + # A lone name ("Madonna", or "Smith" from "Mr. Smith") goes in first_name. first, last = last, nil if first.nil? Result.new(first_name: first, last_name: last) @@ -40,14 +35,12 @@ def run private - # Namae parsed "Jane W Elder" as given "Jane", family "W", title "Elder". - # An honorific at the very end of a name is a surname, not a title. + # "Jane W Elder" parses as given "Jane", family "W", title "Elder". def trailing_word_taken_as_title?(parsed) parsed.title.present? && @full_name.downcase.end_with?(parsed.title.downcase) end - # Namae reads leading lowercase words as particles, so "john smith" comes - # back with no given name and "john" as a particle. + # "john smith" parses with no given name and "john" as a particle. def given_name_taken_as_particle?(parsed) parsed.given.blank? && parsed.particle.present? end diff --git a/app/tasks/maintenance/backfill_user_legal_names_task.rb b/app/tasks/maintenance/backfill_user_legal_names_task.rb new file mode 100644 index 0000000000..7a13dc254a --- /dev/null +++ b/app/tasks/maintenance/backfill_user_legal_names_task.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Maintenance + class BackfillUserLegalNamesTask < MaintenanceTasks::Task + def collection + User.where(legal_first_name: nil).where.not(full_name: [nil, ""]) + end + + def process(user) + parsed = UserService::ParseName.new(full_name: user.full_name).run + + # Scoped to still-unsplit rows so a name the user saved in the meantime is never overwritten. + User.where(id: user.id, legal_first_name: nil, full_name: user.full_name) + .update_all(legal_first_name: parsed.first_name, legal_last_name: parsed.last_name) + end + + end +end diff --git a/app/views/api/v4/users/_user.json.jbuilder b/app/views/api/v4/users/_user.json.jbuilder index 78676c099d..5f4b136d74 100644 --- a/app/views/api/v4/users/_user.json.jbuilder +++ b/app/views/api/v4/users/_user.json.jbuilder @@ -14,8 +14,7 @@ object_shape(json, user, created_at: false) do json.email user.email end - # Legal names stay with the user (or admins with the `pii` scope): org - # members see `name`, which respects the preferred name. + # Other org members only see `name`, which respects the preferred name. expand_pii(override_if: user == current_user) do json.first_name user.first_name(legal: true) json.last_name user.last_name(legal: true) diff --git a/app/views/event/applications/edit.html.erb b/app/views/event/applications/edit.html.erb index ce342ca761..f8bc9ccf07 100644 --- a/app/views/event/applications/edit.html.erb +++ b/app/views/event/applications/edit.html.erb @@ -122,14 +122,14 @@
- <%= form.label :first_name do %> + <%= form.label :legal_first_name do %> Legal first name * <% end %> - <%= form.text_field :first_name, value: @application.user.first_name(legal: true).presence, required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %> + <%= form.text_field :legal_first_name, value: @application.user.first_name(legal: true).presence, required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %>
- <%= form.label :last_name, "Legal last name (optional)" %> - <%= form.text_field :last_name, value: @application.user.last_name(legal: true).presence, placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %> + <%= form.label :legal_last_name, "Legal last name (optional)" %> + <%= form.text_field :legal_last_name, value: @application.user.last_name(legal: true).presence, placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %>
diff --git a/app/views/event/applications/personal_info.html.erb b/app/views/event/applications/personal_info.html.erb index 3b2ec42094..7fd5fc2fb8 100644 --- a/app/views/event/applications/personal_info.html.erb +++ b/app/views/event/applications/personal_info.html.erb @@ -17,14 +17,14 @@
- <%= form.label :first_name do %> + <%= form.label :legal_first_name do %> Legal first name * <% end %> - <%= form.text_field :first_name, value: @application.user.first_name(legal: true).presence, required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %> + <%= form.text_field :legal_first_name, value: @application.user.first_name(legal: true).presence, required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %>
- <%= form.label :last_name, "Legal last name (optional)" %> - <%= form.text_field :last_name, value: @application.user.last_name(legal: true).presence, placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %> + <%= form.label :legal_last_name, "Legal last name (optional)" %> + <%= form.text_field :legal_last_name, value: @application.user.last_name(legal: true).presence, placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %>
diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index fa94f2569a..2fba46406e 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -34,16 +34,16 @@ <%= form_errors @user, "profile", "We couldn't update your" %>
- <%= form.label :first_name %> - <%= form.text_field :first_name, value: @user.first_name(legal: true), required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %> - <% @user.errors.full_messages_for(:first_name).each do |message| %> + <%= form.label :legal_first_name %> + <%= form.text_field :legal_first_name, value: @user.first_name(legal: true), required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %> + <% @user.errors.full_messages_for(:legal_first_name).each do |message| %>
<%= message %>
<% end %>
- <%= form.label :last_name, "Legal last name (optional)" %> - <%= form.text_field :last_name, value: @user.last_name(legal: true), placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %> - <% @user.errors.full_messages_for(:last_name).each do |message| %> + <%= form.label :legal_last_name, "Legal last name (optional)" %> + <%= form.text_field :legal_last_name, value: @user.last_name(legal: true), placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %> + <% @user.errors.full_messages_for(:legal_last_name).each do |message| %>
<%= message %>
<% end %>
@@ -148,18 +148,18 @@
- <%= form.label :first_name %> + <%= form.label :legal_first_name %>

As it appears on your ID. Used for cards and payouts.

- <%= form.text_field :first_name, value: @user.first_name(legal: true), required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name", disabled: %> - <% @user.errors.full_messages_for(:first_name).each do |message| %> + <%= form.text_field :legal_first_name, value: @user.first_name(legal: true), required: true, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name", disabled: %> + <% @user.errors.full_messages_for(:legal_first_name).each do |message| %>
<%= message %>
<% end %>
- <%= form.label :last_name %> + <%= form.label :legal_last_name %>

Optional if you only have one name.

- <%= form.text_field :last_name, value: @user.last_name(legal: true), placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name", disabled: %> - <% @user.errors.full_messages_for(:last_name).each do |message| %> + <%= form.text_field :legal_last_name, value: @user.last_name(legal: true), placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name", disabled: %> + <% @user.errors.full_messages_for(:legal_last_name).each do |message| %>
<%= message %>
<% end %>
diff --git a/app/views/users/first/new.html.erb b/app/views/users/first/new.html.erb index 77389baaf2..809cda4b3c 100644 --- a/app/views/users/first/new.html.erb +++ b/app/views/users/first/new.html.erb @@ -36,12 +36,12 @@

Tell us a little about yourself.

- <%= form.label :first_name, "First name" %> - <%= form.text_field :first_name, value: @user.first_name(legal: true), placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %> + <%= form.label :legal_first_name, "First name" %> + <%= form.text_field :legal_first_name, placeholder: "Fiona", style: "max-width:100%", autocomplete: "given-name" %>
- <%= form.label :last_name, "Last name" %> - <%= form.text_field :last_name, value: @user.last_name(legal: true), placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %> + <%= form.label :legal_last_name, "Last name" %> + <%= form.text_field :legal_last_name, placeholder: "Hackworth", style: "max-width:100%", autocomplete: "family-name" %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index 4dc0ee69e7..c8f98a3b8a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -39,8 +39,6 @@ en: slug: "URL" user: full_name: "Legal name" - first_name: "Legal first name" - last_name: "Legal last name" errors: models: donation: diff --git a/db/migrate/20261102000001_add_first_and_last_names_to_users.rb b/db/migrate/20261102000001_add_first_and_last_names_to_users.rb deleted file mode 100644 index db40c28113..0000000000 --- a/db/migrate/20261102000001_add_first_and_last_names_to_users.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class AddFirstAndLastNamesToUsers < ActiveRecord::Migration[8.1] - def change - add_column :users, :first_name, :string - add_column :users, :last_name, :string - end - -end diff --git a/db/migrate/20261102000001_add_legal_first_and_last_names_to_users.rb b/db/migrate/20261102000001_add_legal_first_and_last_names_to_users.rb new file mode 100644 index 0000000000..451c704115 --- /dev/null +++ b/db/migrate/20261102000001_add_legal_first_and_last_names_to_users.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddLegalFirstAndLastNamesToUsers < ActiveRecord::Migration[8.1] + def change + add_column :users, :legal_first_name, :string + add_column :users, :legal_last_name, :string + end + +end diff --git a/db/migrate/20261102000002_backfill_user_first_and_last_names.rb b/db/migrate/20261102000002_backfill_user_first_and_last_names.rb deleted file mode 100644 index c93c35adc7..0000000000 --- a/db/migrate/20261102000002_backfill_user_first_and_last_names.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -# Splits every existing `users.full_name` into `first_name` / `last_name`. -# Rows that already have a first name are skipped, so this is safe to re-run. -class BackfillUserFirstAndLastNames < ActiveRecord::Migration[8.1] - disable_ddl_transaction! - - BATCH_SIZE = 1_000 - - class MigrationUser < ApplicationRecord - self.table_name = "users" - - end - - def up - MigrationUser.where(first_name: nil).where.not(full_name: [nil, ""]).in_batches(of: BATCH_SIZE) do |batch| - rows = batch.pluck(:id, :full_name).filter_map do |id, full_name| - parsed = UserService::ParseName.new(full_name:).run - next if parsed.first_name.blank? - - [id, parsed.first_name, parsed.last_name] - end - next if rows.empty? - - values = rows.map { |row| MigrationUser.sanitize_sql_array(["(?, ?, ?)", *row]) }.join(", ") - MigrationUser.connection.execute(<<~SQL.squish) - UPDATE users - SET first_name = parsed.first_name, last_name = parsed.last_name - FROM (VALUES #{values}) AS parsed(id, first_name, last_name) - WHERE users.id = parsed.id - SQL - end - end - - def down - # The columns are dropped by the previous migration; nothing to undo here. - end - -end diff --git a/db/schema.rb b/db/schema.rb index eecdcae519..3782a774d5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_11_02_000002) do +ActiveRecord::Schema[8.1].define(version: 2026_11_02_000001) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_catalog.plpgsql" @@ -2968,10 +2968,10 @@ t.integer "creation_method" t.string "discord_id" t.text "email", null: false - t.string "first_name" t.string "full_name" t.boolean "joined_as_teenager" - t.string "last_name" + t.string "legal_first_name" + t.string "legal_last_name" t.datetime "locked_at", precision: nil t.boolean "monthly_donation_summary", default: true t.boolean "monthly_follower_summary", default: true diff --git a/spec/controllers/admin_controller_spec.rb b/spec/controllers/admin_controller_spec.rb index 7f58ddbb61..86fdf17e4c 100644 --- a/spec/controllers/admin_controller_spec.rb +++ b/spec/controllers/admin_controller_spec.rb @@ -191,7 +191,7 @@ it "labels nameless and space-padded users with trimmed text" do admin = create(:user, :make_admin) create_session(admin, verified: true) - nameless = create(:user, first_name: nil, last_name: nil) + nameless = create(:user, legal_first_name: nil, legal_last_name: nil) padded = create(:user, full_name: "Jane Doe ") get(:user_search, format: :json) diff --git a/spec/controllers/api/v4/events_controller_spec.rb b/spec/controllers/api/v4/events_controller_spec.rb index 98c5719090..1f005bb85f 100644 --- a/spec/controllers/api/v4/events_controller_spec.rb +++ b/spec/controllers/api/v4/events_controller_spec.rb @@ -29,7 +29,7 @@ it "does not expose another organizer's legal name over their preferred name" do current_user = create(:user) - other_user = create(:user, first_name: "Zebulon", last_name: "Quixley", preferred_name: "Zeb Q") + other_user = create(:user, legal_first_name: "Zebulon", legal_last_name: "Quixley", preferred_name: "Zeb Q") event = create(:event) create(:organizer_position, user: current_user, event:) create(:organizer_position, user: other_user, event:) @@ -44,8 +44,26 @@ expect(response.body).not_to include("Zebulon", "Quixley") end + it "shows another organizer without a preferred name by first name and last initial only" do + current_user = create(:user) + other_user = create(:user, legal_first_name: "Zebulon Xavier", legal_last_name: "Quixley") + event = create(:event) + create(:organizer_position, user: current_user, event:) + create(:organizer_position, user: other_user, event:) + + token = create(:api_token, user: current_user) + request.headers["Authorization"] = "Bearer #{token.token}" + + get :show, params: { id: event.friendly_id, expand: "users" }, as: :json + + other_user_data = response.parsed_body["users"].find { |u| u["id"] == other_user.public_id } + expect(other_user_data).to include("name" => "Zebulon Q") + expect(other_user_data).not_to include("first_name", "last_name") + expect(response.body).not_to include("Xavier", "Quixley") + end + it "exposes the current user's own email and legal name" do - current_user = create(:user, email: "me@example.com", first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi") + current_user = create(:user, email: "me@example.com", legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "FiFi") event = create(:event) create(:organizer_position, user: current_user, event:) diff --git a/spec/controllers/logins_controller_spec.rb b/spec/controllers/logins_controller_spec.rb index 5587881842..44287054a8 100644 --- a/spec/controllers/logins_controller_spec.rb +++ b/spec/controllers/logins_controller_spec.rb @@ -562,7 +562,7 @@ end it "redirects to the user's settings page if they don't have a name or phone number" do - user = create(:user, first_name: nil, last_name: nil, phone_number: nil) + user = create(:user, legal_first_name: nil, legal_last_name: nil, phone_number: nil) login = create(:login, user:) login_code = create(:login_code, user:) diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 9409334435..8bc68831e5 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -248,37 +248,37 @@ render_views it "asks a new user for their first and last name during onboarding" do - user = create(:user, first_name: nil, last_name: nil) + user = create(:user, legal_first_name: nil, legal_last_name: nil) create_session(user, verified: true) get(:edit, params: { id: user.id }) expect(response).to have_http_status(:ok) expect(response.body).to include("Create your account") - expect(response.body).to include('name="user[first_name]"') - expect(response.body).to include('name="user[last_name]"') + expect(response.body).to include('name="user[legal_first_name]"') + expect(response.body).to include('name="user[legal_last_name]"') expect(response.body).not_to include('name="user[full_name]"') end it "shows the stored legal name in settings, not the preferred name" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "FiFi") create_session(user, verified: true) get(:edit, params: { id: user.id }) - expect(response.body).to include('name="user[first_name]"').and include('value="Fiona"') - expect(response.body).to include('name="user[last_name]"').and include('value="Hackworth"') + expect(response.body).to include('name="user[legal_first_name]"').and include('value="Fiona"') + expect(response.body).to include('name="user[legal_last_name]"').and include('value="Hackworth"') end it "leaves the last name blank for a user with a multi-word first name and no last name" do - user = create(:user, first_name: "Mary Ann", last_name: nil, phone_number: "+18556254225") + user = create(:user, legal_first_name: "Mary Ann", legal_last_name: nil, phone_number: "+18556254225") create_session(user, verified: true) get(:edit, params: { id: user.id }) page = Nokogiri::HTML(response.body) - expect(page.at_css("input[name='user[first_name]']")["value"]).to eq("Mary Ann") - expect(page.at_css("input[name='user[last_name]']")["value"]).to be_blank + expect(page.at_css("input[name='user[legal_first_name]']")["value"]).to eq("Mary Ann") + expect(page.at_css("input[name='user[legal_last_name]']")["value"]).to be_blank end end @@ -410,43 +410,43 @@ end it "completes onboarding once the user enters their name" do - user = create(:user, first_name: nil, last_name: nil) + user = create(:user, legal_first_name: nil, legal_last_name: nil) create_session(user, verified: true) - patch(:update, params: { id: user.id, user: { first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi" } }) + patch(:update, params: { id: user.id, user: { legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "FiFi" } }) expect(response).to redirect_to(root_path) expect(flash[:success]).to eq("Profile created!") - expect(user.reload.attributes).to include("first_name" => "Fiona", "last_name" => "Hackworth", "full_name" => "Fiona Hackworth", "preferred_name" => "FiFi") + expect(user.reload.attributes).to include("legal_first_name" => "Fiona", "legal_last_name" => "Hackworth", "full_name" => "Fiona Hackworth", "preferred_name" => "FiFi") expect(user).not_to be_onboarding end it "lets a user go by a first name only" do - user = create(:user, first_name: nil, last_name: nil) + user = create(:user, legal_first_name: nil, legal_last_name: nil) create_session(user, verified: true) - patch(:update, params: { id: user.id, user: { first_name: "Madonna", last_name: "" } }) + patch(:update, params: { id: user.id, user: { legal_first_name: "Madonna", legal_last_name: "" } }) expect(response).to redirect_to(root_path) - expect(user.reload).to have_attributes(first_name: "Madonna", last_name: nil, full_name: "Madonna") + expect(user.reload).to have_attributes(legal_first_name: "Madonna", legal_last_name: nil, full_name: "Madonna") end it "updates the user's name" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") create_session(user, verified: true) - patch(:update, params: { id: user.id, user: { first_name: "Jane", last_name: "Elder" } }) + patch(:update, params: { id: user.id, user: { legal_first_name: "Jane", legal_last_name: "Elder" } }) expect(response).to redirect_to(edit_user_path(user)) expect(flash[:success]).to eq("Updated your profile!") - expect(user.reload).to have_attributes(first_name: "Jane", last_name: "Elder", full_name: "Jane Elder") + expect(user.reload).to have_attributes(legal_first_name: "Jane", legal_last_name: "Elder", full_name: "Jane Elder") end it "does not let the first name be removed" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") create_session(user, verified: true) - patch(:update, params: { id: user.id, user: { first_name: "", last_name: "Hackworth" } }) + patch(:update, params: { id: user.id, user: { legal_first_name: "", legal_last_name: "Hackworth" } }) expect(response).to have_http_status(:unprocessable_content) expect(response.body).to include("Legal first name can't be blank") @@ -454,24 +454,49 @@ end it "rejects names with unsupported characters" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") create_session(user, verified: true) - patch(:update, params: { id: user.id, user: { first_name: "Fiona", last_name: "Hackworth [Dev]" } }) + patch(:update, params: { id: user.id, user: { legal_first_name: "Fiona", legal_last_name: "Hackworth [Dev]" } }) expect(response).to have_http_status(:unprocessable_content) expect(response.body).to include("Legal last name may only contain letters") expect(user.reload.last_name).to eq("Hackworth") end + it "does not let a cardholder remove their last name" do + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") + create(:stripe_cardholder, user:) + create_session(user, verified: true) + + patch(:update, params: { id: user.id, user: { legal_first_name: "Fiona", legal_last_name: "" } }) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include("Legal last name is required for HCB cards") + expect(user.reload.full_name).to eq("Fiona Hackworth") + end + + it "explains the missing last name instead of erroring when a cardholder has none" do + user = create(:user, legal_first_name: "Madonna", legal_last_name: nil, phone_number: "+18556254225") + create(:stripe_cardholder, user:) + create_session(user, verified: true) + expect(StripeService::Issuing::Cardholder).not_to receive(:update) + + patch(:update, params: { id: user.id, user: { legal_first_name: "Madonna Louise", legal_last_name: "" } }) + + expect(response).to redirect_to(edit_user_path(user)) + expect(flash[:error]).to eq("A legal last name is required to issue cards. Please add it in your account settings.") + expect(user.reload.full_name).to eq("Madonna Louise") + end + it "ignores the legacy full_name parameter" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") create_session(user, verified: true) patch(:update, params: { id: user.id, user: { full_name: "Someone Else" } }) expect(response).to have_http_status(:found) - expect(user.reload).to have_attributes(first_name: "Fiona", last_name: "Hackworth", full_name: "Fiona Hackworth") + expect(user.reload).to have_attributes(legal_first_name: "Fiona", legal_last_name: "Hackworth", full_name: "Fiona Hackworth") end it "lets an admin bypass phone number verification for a user" do diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 191bbb3d0a..5a08362d1e 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -3,11 +3,10 @@ FactoryBot.define do factory :user do sequence(:email) { |n| "user#{n}@example.com" } - # Passing `full_name:` is still supported: the model splits it into - # `first_name` / `last_name` on validation, so the defaults step aside. + # A given `full_name:` is split by the model, so the defaults step aside. full_name { nil } - first_name { full_name.present? ? nil : Faker::Name.first_name } - last_name { full_name.present? ? nil : Faker::Name.last_name } + legal_first_name { full_name.present? ? nil : Faker::Name.first_name } + legal_last_name { full_name.present? ? nil : Faker::Name.last_name } session_validity_preference { SessionsHelper::SESSION_DURATION_OPTIONS.fetch("3 days") } verified { true } diff --git a/spec/misc/flipper_spec.rb b/spec/misc/flipper_spec.rb index a750a9690f..6503c1ce1a 100644 --- a/spec/misc/flipper_spec.rb +++ b/spec/misc/flipper_spec.rb @@ -21,7 +21,7 @@ end it "handles users without names" do - user = create(:user, first_name: nil, last_name: nil, email: "orpheus@hackclub.com") + user = create(:user, legal_first_name: nil, legal_last_name: nil, email: "orpheus@hackclub.com") actor_names = Flipper::UI.configuration.actor_names_source.call([user.flipper_id]) @@ -30,7 +30,7 @@ it "ignores unknown or no longer existent keys" do event = create(:event, name: "Hack the Planet") - user = create(:user, first_name: nil, last_name: nil, email: "orpheus@hackclub.com") + user = create(:user, legal_first_name: nil, legal_last_name: nil, email: "orpheus@hackclub.com") deleted_event = create(:event) deleted_event_flipper_id = deleted_event.flipper_id diff --git a/spec/models/stripe_cardholder_spec.rb b/spec/models/stripe_cardholder_spec.rb index f7938e5fd9..4aa39d6b28 100644 --- a/spec/models/stripe_cardholder_spec.rb +++ b/spec/models/stripe_cardholder_spec.rb @@ -85,14 +85,14 @@ describe ".first_name and .last_name" do it "use the user's legal name, not their preferred name" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi Hacks") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "FiFi Hacks") expect(StripeCardholder.first_name(user)).to eq("Fiona") expect(StripeCardholder.last_name(user)).to eq("Hackworth") end it "keep middle names and suffixes" do - user = create(:user, first_name: "Martin Luther", last_name: "King Jr.") + user = create(:user, legal_first_name: "Martin Luther", legal_last_name: "King Jr.") expect(StripeCardholder.first_name(user)).to eq("Martin Luther") expect(StripeCardholder.last_name(user)).to eq("King Jr.") @@ -106,14 +106,14 @@ end it "transliterate accented characters" do - user = create(:user, first_name: "María-José", last_name: "Carreño Quiñones") + user = create(:user, legal_first_name: "María-José", legal_last_name: "Carreño Quiñones") expect(StripeCardholder.first_name(user)).to eq("Maria-Jose") expect(StripeCardholder.last_name(user)).to eq("Carreno Quinones") end it "explain that a last name is required when the user only has a first name" do - user = create(:user, first_name: "Madonna", last_name: nil) + user = create(:user, legal_first_name: "Madonna", legal_last_name: nil) expect(StripeCardholder.first_name(user)).to eq("Madonna") expect { StripeCardholder.last_name(user) }.to raise_error( @@ -123,14 +123,14 @@ end it "explain that a first name is required when the user has no name" do - user = create(:user, first_name: nil, last_name: nil) + user = create(:user, legal_first_name: nil, legal_last_name: nil) expect { StripeCardholder.first_name(user) }.to raise_error(Errors::StripeInvalidNameError, /legal first name is required/) end it "reject names with no latin letters" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") - user.update_columns(first_name: "ジョン", full_name: "ジョン Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") + user.update_columns(legal_first_name: "ジョン", full_name: "ジョン Hackworth") expect { StripeCardholder.first_name(user) }.to raise_error(Errors::StripeInvalidNameError, /must contain at least 1 letter/) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 799659127b..15b1abe9c7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -98,40 +98,45 @@ describe "name storage" do it "keeps full_name in sync with first_name and last_name" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") expect(user.full_name).to eq("Fiona Hackworth") - user.update!(last_name: "Hackworth-Smith") + user.update!(legal_last_name: "Hackworth-Smith") expect(user.reload.full_name).to eq("Fiona Hackworth-Smith") end it "stores a first name on its own" do - user = create(:user, first_name: "Madonna", last_name: nil) + user = create(:user, legal_first_name: "Madonna", legal_last_name: nil) expect(user.full_name).to eq("Madonna") expect(user.last_name).to be_nil end it "does not derive a last name from a multi-word first name" do - user = create(:user, first_name: "Mary Ann", last_name: nil).reload + user = create(:user, legal_first_name: "Mary Ann", legal_last_name: nil).reload expect(user.first_name(legal: true)).to eq("Mary Ann") expect(user.last_name(legal: true)).to be_nil expect(user.last_name).to be_nil - expect(user.initial_name).to eq("Mary Ann") + end + + it "shows others only the first word when there is no last name" do + user = create(:user, legal_first_name: "Mary Jane Smith", legal_last_name: nil) + + expect(user.initial_name).to eq("Mary") end it "initials the surname, not its lowercase particle" do - user = create(:user, first_name: "Juan", last_name: "de la Cruz") + user = create(:user, legal_first_name: "Juan", legal_last_name: "de la Cruz") expect(user.initial_name).to eq("Juan C") expect(user.first_name).to eq("Juan") end it "does not refill a first name that was just cleared" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") - expect(user.update(first_name: "")).to be(false) + expect(user.update(legal_first_name: "")).to be(false) expect(user.first_name(legal: true)).to be_nil end @@ -147,30 +152,42 @@ expect(user.full_name).to eq("Martin Luther King Jr.") end + it "keeps every word of a full_name holding several people" do + user = create(:user, full_name: "John and Jane Doe") + + expect(user.reload).to have_attributes(legal_first_name: "John and Jane", legal_last_name: "Doe", full_name: "John and Jane Doe") + end + + it "serializes the legal name columns, not the preferred name" do + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "FiFi Hacks") + + expect(user.as_json(only: [:legal_first_name, :legal_last_name])).to eq("legal_first_name" => "Fiona", "legal_last_name" => "Hackworth") + end + it "canonicalizes a comma-form full_name" do user = create(:user, full_name: "Doe, John") - expect(user.reload).to have_attributes(first_name: "John", last_name: "Doe", full_name: "John Doe") + expect(user.reload).to have_attributes(legal_first_name: "John", legal_last_name: "Doe", full_name: "John Doe") end it "re-splits when a legacy writer changes full_name" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") user.update!(full_name: "Jane Elder") - expect(user.reload).to have_attributes(first_name: "Jane", last_name: "Elder", full_name: "Jane Elder") + expect(user.reload).to have_attributes(legal_first_name: "Jane", legal_last_name: "Elder", full_name: "Jane Elder") end it "prefers the split names when both are assigned at once" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") - user.update!(first_name: "Jane", last_name: "Elder", full_name: "Someone Else") + user.update!(legal_first_name: "Jane", legal_last_name: "Elder", full_name: "Someone Else") - expect(user.reload).to have_attributes(first_name: "Jane", last_name: "Elder", full_name: "Jane Elder") + expect(user.reload).to have_attributes(legal_first_name: "Jane", legal_last_name: "Elder", full_name: "Jane Elder") end it "trims and collapses whitespace" do - user = create(:user, first_name: " Fiona Rose ", last_name: " Hackworth ") + user = create(:user, legal_first_name: " Fiona Rose ", legal_last_name: " Hackworth ") expect(user.first_name(legal: true)).to eq("Fiona Rose") expect(user.last_name(legal: true)).to eq("Hackworth") @@ -178,25 +195,25 @@ end it "treats a blank last name as no last name" do - user = create(:user, first_name: "Fiona", last_name: " ") + user = create(:user, legal_first_name: "Fiona", legal_last_name: " ") expect(user.last_name).to be_nil expect(user.full_name).to eq("Fiona") end it "updates the personal legal entity's name" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") expect(user.personal_legal_entity.name).to eq("Fiona Hackworth") - user.update!(first_name: "Jane", last_name: "Elder") + user.update!(legal_first_name: "Jane", legal_last_name: "Elder") expect(user.personal_legal_entity.reload.name).to eq("Jane Elder") end context "for a row that predates the split columns" do let(:user) do - create(:user, first_name: "Fiona", last_name: "Hackworth").tap do |u| - u.update_columns(first_name: nil, last_name: nil, full_name: "Jane Elder") + create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth").tap do |u| + u.update_columns(legal_first_name: nil, legal_last_name: nil, full_name: "Jane Elder") u.reload end end @@ -210,41 +227,72 @@ it "still allows unrelated updates without touching the name" do expect(user.update(phone_number: "+18556254225")).to be(true) - expect(user.reload.first_name_in_database).to be_nil + expect(user.reload.legal_first_name_in_database).to be_nil expect(user.full_name).to eq("Jane Elder") end it "does not allow the name to be removed" do - user.update(first_name: "", last_name: "") + user.update(legal_first_name: "", legal_last_name: "") + + expect(user.reload.full_name).to eq("Jane Elder") + expect(user.first_name(legal: true)).to eq("Jane") + expect(user.last_name(legal: true)).to eq("Elder") + end + + it "keeps the surname when only the first name is edited" do + user.update!(legal_first_name: "Janet") + + expect(user.reload).to have_attributes(legal_first_name: "Janet", legal_last_name: "Elder", full_name: "Janet Elder") + end + + it "keeps the first name when only the surname is edited" do + user.update!(legal_last_name: "Smith") + + expect(user.reload).to have_attributes(legal_first_name: "Jane", legal_last_name: "Smith", full_name: "Jane Smith") + end + + it "does not let a cardholder drop their surname by blanking it" do + create(:stripe_cardholder, user:) + + user.update(legal_first_name: "Jane", legal_last_name: "") expect(user.reload.full_name).to eq("Jane Elder") + expect(StripeCardholder.last_name(user)).to eq("Elder") + end + + it "keeps a title in full_name when the parsed name is saved back unchanged" do + user.update_columns(full_name: "Dr. Juan de la Cruz") + + user.reload.update!(legal_first_name: "Juan", legal_last_name: "de la Cruz", phone_number: "+18556254225") + + expect(user.reload).to have_attributes(legal_first_name: "Juan", legal_last_name: "de la Cruz", full_name: "Dr. Juan de la Cruz") end it "backfills the columns once the name is edited" do - user.update!(first_name: "Jane", last_name: "Elder") + user.update!(legal_first_name: "Jane", legal_last_name: "Elder") - expect(user.reload).to have_attributes(first_name: "Jane", last_name: "Elder", full_name: "Jane Elder") + expect(user.reload).to have_attributes(legal_first_name: "Jane", legal_last_name: "Elder", full_name: "Jane Elder") end end end describe "name validations" do it "allows a user without a name" do - expect(build(:user, first_name: nil, last_name: nil)).to be_valid + expect(build(:user, legal_first_name: nil, legal_last_name: nil)).to be_valid end it "requires a first name when there is a last name" do - user = build(:user, first_name: nil, last_name: "Hackworth") + user = build(:user, legal_first_name: nil, legal_last_name: "Hackworth") expect(user).not_to be_valid - expect(user.errors[:first_name]).to include("can't be blank") + expect(user.errors[:legal_first_name]).to include("can't be blank") end it "does not allow a name to be removed once set" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") - expect(user.update(first_name: "", last_name: "")).to be(false) - expect(user.errors[:first_name]).to include("can't be blank") + expect(user.update(legal_first_name: "", legal_last_name: "")).to be(false) + expect(user.errors[:legal_first_name]).to include("can't be blank") expect(user.reload.full_name).to eq("Fiona Hackworth") end @@ -252,34 +300,34 @@ user = create(:user, full_name: "Caleb Denio") expect(user.update(full_name: "")).to be(false) - expect(user.errors[:first_name]).to include("can't be blank") + expect(user.errors[:legal_first_name]).to include("can't be blank") end it "allows the last name to be removed" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") - expect(user.update(last_name: "")).to be(true) + expect(user.update(legal_last_name: "")).to be(true) expect(user.reload.full_name).to eq("Fiona") end it "does not allow a cardholder to remove their last name" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth") create(:stripe_cardholder, user:) - expect(user.update(last_name: "")).to be(false) - expect(user.errors[:last_name]).to include("is required for HCB cards") + expect(user.update(legal_last_name: "")).to be(false) + expect(user.errors[:legal_last_name]).to include("is required for HCB cards") expect(user.reload.full_name).to eq("Fiona Hackworth") end it "lets a cardholder with no last name on file update other details" do - user = create(:user, first_name: "Madonna", last_name: nil) + user = create(:user, legal_first_name: "Madonna", legal_last_name: nil) create(:stripe_cardholder, user:) - expect(user.reload.update(first_name: "Madonna Louise")).to be(true) + expect(user.reload.update(legal_first_name: "Madonna Louise")).to be(true) end it "accepts accented letters and name punctuation" do - user = build(:user, first_name: "María-José", last_name: "O'Brien, Jr.") + user = build(:user, legal_first_name: "María-José", legal_last_name: "O'Brien, Jr.") expect(user).to be_valid end @@ -291,10 +339,10 @@ "digits" => ["5512700050241863", nil], }.each do |label, (first, last)| it "rejects #{label}" do - user = build(:user, first_name: first, last_name: last) + user = build(:user, legal_first_name: first, legal_last_name: last) expect(user).not_to be_valid - expect(user.errors[:first_name] + user.errors[:last_name]).to include(User::NAME_CHARACTERS_MESSAGE) + expect(user.errors[:legal_first_name] + user.errors[:legal_last_name]).to include(User::NAME_CHARACTERS_MESSAGE) end end @@ -302,14 +350,14 @@ user = build(:user, full_name: "Zach Latta [Dev]") expect(user).not_to be_valid - expect(user.errors[:last_name]).to include(User::NAME_CHARACTERS_MESSAGE) + expect(user.errors[:legal_last_name]).to include(User::NAME_CHARACTERS_MESSAGE) end end describe "#initials" do context "when missing name" do it "returns initials from email" do - user = create(:user, email: "user1@example.com", first_name: nil, last_name: nil) + user = create(:user, email: "user1@example.com", legal_first_name: nil, legal_last_name: nil) expect(user.initials).to eql("U") end @@ -318,17 +366,23 @@ describe "#safe_name" do it "returns the preferred name when it fits" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "FiFi") expect(user.safe_name).to eql("FiFi") end it "falls back to the legal name when the preferred name is too long" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "Fiona the Magnificent Hacker") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "Fiona the Magnificent Hacker") expect(user.safe_name).to eql("Fiona Hackworth") end + it "never exceeds 24 chars, even for a long single name" do + user = create(:user, legal_first_name: "Thisisareallyreallylongsinglename", legal_last_name: nil) + + expect(user.safe_name).to eql("Thisisareallyreallylongs") + end + context "when initial name is really long" do it "returns safe_name max of 24 chars" do user = create(:user, full_name: "Thisisareallyreallylongfirstnamethatembursewillnotlike Last") @@ -341,7 +395,7 @@ describe "#first_name and #last_name" do it "return the stored names, with only the first word for display" do - user = create(:user, first_name: "Donald Ervin", last_name: "Knuth") + user = create(:user, legal_first_name: "Donald Ervin", legal_last_name: "Knuth") expect(user.first_name).to eql("Donald") expect(user.last_name).to eql("Knuth") @@ -350,7 +404,7 @@ end it "return nil for a user without a name, except the display first name" do - user = create(:user, email: "fiona.h@example.com", first_name: nil, last_name: nil) + user = create(:user, email: "fiona.h@example.com", legal_first_name: nil, legal_last_name: nil) expect(user.first_name).to eq("fiona.h") expect(user.last_name).to be_nil @@ -359,7 +413,7 @@ end context "with a preferred name" do - let(:user) { create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi Hacks") } + let(:user) { create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "FiFi Hacks") } it "favor the preferred name" do expect(user.first_name).to eql("FiFi") @@ -383,38 +437,38 @@ describe "#initial_name" do it "returns the first name and last initial" do - user = create(:user, first_name: "First", last_name: "Last") + user = create(:user, legal_first_name: "First", legal_last_name: "Last") expect(user.initial_name).to eql("First L") end it "uses only the first word of a multi-word first name" do - user = create(:user, first_name: "Martin Luther", last_name: "King Jr.") + user = create(:user, legal_first_name: "Martin Luther", legal_last_name: "King Jr.") expect(user.initial_name).to eql("Martin K") end it "uses the preferred name" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi Hacks") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "FiFi Hacks") expect(user.initial_name).to eql("FiFi H") end it "returns a single-word preferred name as is" do - user = create(:user, first_name: "Fiona", last_name: "Hackworth", preferred_name: "FiFi") + user = create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "FiFi") expect(user.initial_name).to eql("FiFi") end it "returns a lone first name as is" do - user = create(:user, first_name: "Madonna", last_name: nil) + user = create(:user, legal_first_name: "Madonna", legal_last_name: nil) expect(user.initial_name).to eql("Madonna") end context "when the user has no name" do it "returns the email handle" do - user = create(:user, email: "user1@example.com", first_name: nil, last_name: nil) + user = create(:user, email: "user1@example.com", legal_first_name: nil, legal_last_name: nil) expect(user.initial_name).to eql("user1") end @@ -423,10 +477,10 @@ describe "onboarding" do it "ends once a first name is saved" do - user = create(:user, first_name: nil, last_name: nil) + user = create(:user, legal_first_name: nil, legal_last_name: nil) expect(user).to be_onboarding - user.update!(first_name: "Fiona") + user.update!(legal_first_name: "Fiona") expect(user).not_to be_onboarding expect(user).to be_was_onboarding diff --git a/spec/requests/event/applications_controller_spec.rb b/spec/requests/event/applications_controller_spec.rb new file mode 100644 index 0000000000..ae54d5d3eb --- /dev/null +++ b/spec/requests/event/applications_controller_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Event::ApplicationsController, type: :request do + let(:user) { create(:user, legal_first_name: "Fiona", legal_last_name: "Hackworth", preferred_name: "FiFi Hacks") } + let(:application) { create(:event_application, user:) } + + def sign_in_as(user) + allow_any_instance_of(SessionsHelper) + .to receive(:find_current_session) + .and_return(create(:user_session, user:)) + end + + describe "GET /applications/:id/personal_info" do + it "prefills the legal name, not the preferred name" do + sign_in_as(user) + + get personal_info_application_path(application) + + page = Nokogiri::HTML(response.body) + expect(page.at_css("input[name='event_application[legal_first_name]']")["value"]).to eq("Fiona") + expect(page.at_css("input[name='event_application[legal_last_name]']")["value"]).to eq("Hackworth") + expect(page.at_css("input[name='event_application[full_name]']")).to be_nil + end + end + + describe "PATCH /applications/:id" do + it "saves the applicant's legal name" do + sign_in_as(user) + + patch application_path(application), params: { event_application: { legal_first_name: "Jane", legal_last_name: "Elder" }, return_to: application_path(application) } + + expect(response).to redirect_to(application_path(application)) + expect(user.reload).to have_attributes(legal_first_name: "Jane", legal_last_name: "Elder", full_name: "Jane Elder") + end + + it "ignores the legacy full_name parameter" do + sign_in_as(user) + + patch application_path(application), params: { event_application: { full_name: "Someone Else" }, return_to: application_path(application) } + + expect(response).to redirect_to(application_path(application)) + expect(user.reload).to have_attributes(legal_first_name: "Fiona", legal_last_name: "Hackworth", full_name: "Fiona Hackworth") + end + + it "rejects a name with unsupported characters without saving it" do + sign_in_as(user) + + patch application_path(application), params: { event_application: { legal_first_name: "Fiona