Skip to content

Commit

Permalink
Refactor User#temp_username (forem#19252)
Browse files Browse the repository at this point in the history
* Refactor User#temp_username

* Remove Authentication::Providers dependency from Users::UsernameGenerator

* Adds brief description and yard docs
  • Loading branch information
gorangorun authored Mar 27, 2023
1 parent 15bb2e5 commit 0b4a360
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 24 deletions.
32 changes: 9 additions & 23 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -569,33 +569,19 @@ def send_welcome_notification
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
end

def temp_name_exists?
User.exists?(username: temp_username) || Organization.exists?(slug: temp_username)
def auth_provider_usernames
attributes
.with_indifferent_access
.slice(*Authentication::Providers.username_fields)
.values.compact || []
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
Expand Down
52 changes: 52 additions & 0 deletions app/services/users/username_generator.rb
Original file line number Diff line number Diff line change
@@ -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<String>] 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
8 changes: 7 additions & 1 deletion spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,18 @@ def provider_username(service_name)
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
Expand Down
31 changes: 31 additions & 0 deletions spec/services/users/username_generator_spec.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 0b4a360

Please sign in to comment.