diff --git a/app/models/user.rb b/app/models/user.rb index e945f5e4f..f566ba8ff 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -569,33 +569,19 @@ class User < ApplicationRecord end def set_username - set_temp_username if username.blank? - self.username = username&.downcase + self.username = username&.downcase&.presence || generate_username end - # @todo Should we do something to ensure that we don't create a username that violates our - # USERNAME_MAX_LENGTH constant? - # - # @see USERNAME_MAX_LENGTH - def set_temp_username - self.username = if temp_name_exists? - "#{temp_username}_#{rand(100)}" - else - temp_username - end + def auth_provider_usernames + attributes + .with_indifferent_access + .slice(*Authentication::Providers.username_fields) + .values.compact || [] end - def temp_name_exists? - User.exists?(username: temp_username) || Organization.exists?(slug: temp_username) - end - - def temp_username - Authentication::Providers.username_fields.each do |username_field| - value = public_send(username_field) - next if value.blank? - - return value.downcase.gsub(/[^0-9a-z_]/i, "").delete(" ") - end + def generate_username + Users::UsernameGenerator + .call(auth_provider_usernames) end def downcase_email diff --git a/app/services/users/username_generator.rb b/app/services/users/username_generator.rb new file mode 100644 index 000000000..f6c3eacd7 --- /dev/null +++ b/app/services/users/username_generator.rb @@ -0,0 +1,52 @@ +# Generates available username based on +# multiple generators in the following order: +# * list of supplied usernames +# * list of supplied usernames with suffix +# * random generated letters +# +# @todo Extract username validation in separate class +module Users + class UsernameGenerator + def self.call(...) + new(...).call + end + + # @param list [Array] a list of usernames + def initialize(list = []) + @list = list + end + + def call + from_list(modified_list) || from_list(list_with_suffix) || from_list(Array.new(3) { random_letters }) + end + + private + + def from_list(list) + list.detect { |username| !username_exists?(username) } + end + + def username_exists?(username) + User.exists?(username: username) || + Organization.exists?(slug: username) || + Page.exists?(slug: username) || + Podcast.exists?(slug: username) + end + + def filtered_list + @list.select { |s| s.is_a?(String) && s.present? } + end + + def modified_list + filtered_list.map { |s| s.downcase.gsub(/[^0-9a-z_]/i, "").delete(" ") } + end + + def list_with_suffix + modified_list.map { |s| [s, rand(100)].join("_") } + end + + def random_letters + ("a".."z").to_a.sample(12).join + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index dcb1bb726..b424e57ea 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -313,12 +313,18 @@ RSpec.describe User do end describe "#username" do - it "receives a temporary username if none is given" do + it "receives a generated username if none is given" do user.username = "" user.validate! expect(user.username).not_to be_blank end + it "is not valid if generate_username returns nil" do + user.username = "" + allow(user).to receive(:generate_username).and_return(nil) + expect(user).not_to be_valid + end + it "does not allow to change to a username that is taken" do user.username = other_user.username expect(user).not_to be_valid diff --git a/spec/services/users/username_generator_spec.rb b/spec/services/users/username_generator_spec.rb new file mode 100644 index 000000000..9cb233a2b --- /dev/null +++ b/spec/services/users/username_generator_spec.rb @@ -0,0 +1,31 @@ +require "rails_helper" + +RSpec.describe Users::UsernameGenerator, type: :service do + let(:user) { create(:user) } + + it "returns randomly generated username if empty list is passed" do + expect(described_class.call([""])).to be_present + end + + it "returns supplied username if does not exist" do + expect(described_class.call(["foo"])).to eq("foo") + end + + it "returns modified username" do + expect(described_class.call(["foo.bar"])).to eq("foobar") + end + + it "returns supplied username with suffix if exists" do + expect(described_class.call([user.username])).to start_with("#{user.username}_") + end + + it "returns randomly generated username" do + expect(described_class.call).to be_present + end + + it "returns nil if all generation methods are exhausted" do + username_generator = described_class.new + allow(username_generator).to receive(:random_letters).and_return(user.username) + expect(username_generator.call).to be_nil + end +end