Skip to content

Commit a2a2f3f

Browse files
committed
Improved alias addition form
Now users can specify display names, and we also properly add an error when they don't specify a display name when they should
1 parent 8e11d35 commit a2a2f3f

5 files changed

Lines changed: 198 additions & 7 deletions

File tree

app/assets/stylesheets/components/form.css

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,61 @@ input {
1818
input[type=submit], input[type=button] {
1919
background-color: var(--color-bg-button);
2020
}
21+
22+
/* Flash messages */
23+
.flash {
24+
padding: var(--spacing-3) var(--spacing-4);
25+
margin-bottom: var(--spacing-4);
26+
border-radius: var(--border-radius-md);
27+
font-size: var(--font-size-base);
28+
line-height: var(--line-height-normal);
29+
display: flex;
30+
align-items: center;
31+
gap: var(--spacing-2);
32+
33+
&::before {
34+
font-family: "Font Awesome 6 Free";
35+
font-weight: 900;
36+
font-size: var(--font-size-md);
37+
}
38+
}
39+
40+
.flash.alert {
41+
background-color: var(--color-danger-soft);
42+
color: var(--color-danger);
43+
border: var(--border-width) solid var(--color-danger);
44+
45+
&::before {
46+
content: "\f071"; /* fa-triangle-exclamation */
47+
}
48+
}
49+
50+
.flash.notice {
51+
background-color: var(--color-success-soft);
52+
color: var(--color-success);
53+
border: var(--border-width) solid var(--color-success);
54+
55+
&::before {
56+
content: "\f00c"; /* fa-check */
57+
}
58+
}
59+
60+
.flash.warning {
61+
background-color: var(--color-warning-bg);
62+
color: var(--color-warning-text);
63+
border: var(--border-width) solid var(--color-warning);
64+
65+
&::before {
66+
content: "\f06a"; /* fa-circle-exclamation */
67+
}
68+
}
69+
70+
.flash.info {
71+
background-color: var(--color-info-soft);
72+
color: var(--color-info);
73+
border: var(--border-width) solid var(--color-info);
74+
75+
&::before {
76+
content: "\f05a"; /* fa-circle-info */
77+
}
78+
}

app/controllers/settings/emails_controller.rb

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,33 @@ module Settings
44
class EmailsController < Settings::BaseController
55
def create
66
email = EmailNormalizer.normalize(params[:email])
7+
name = params[:name].presence
8+
79
if Alias.by_email(email).where.not(user_id: [nil, current_user.id]).exists?
810
return redirect_to settings_account_path, alert: 'Email is linked to another account. Delete that account first to release it.'
911
end
10-
token, raw = UserToken.issue!(purpose: 'add_alias', user: current_user, email: email, ttl: 1.hour)
12+
13+
existing_in_db = Alias.by_email(email)
14+
user_has_verified = current_user.aliases.by_email(email).where.not(verified_at: nil).exists?
15+
16+
# Case 1: No aliases exist in DB for this email - require a name
17+
if !existing_in_db.exists? && name.blank?
18+
return redirect_to settings_account_path, alert: 'Please provide a display name for this new email address.'
19+
end
20+
21+
# Case 2: User already has verified alias with this email - require a name to create a new alias
22+
if user_has_verified
23+
if name.blank?
24+
return redirect_to settings_account_path, alert: 'This email is already verified. Please provide a display name to add a new alias.'
25+
end
26+
person = current_user.person
27+
Alias.create!(person: person, user: current_user, name: name, email: email, verified_at: Time.current)
28+
return redirect_to settings_account_path, notice: 'Alias added.'
29+
end
30+
31+
# Case 3: Email exists in DB but not associated with user - send verification
32+
metadata = { name: name }.to_json if name.present?
33+
token, raw = UserToken.issue!(purpose: 'add_alias', user: current_user, email: email, ttl: 1.hour, metadata: metadata)
1134
UserMailer.verification_email(token, raw).deliver_later
1235
redirect_to settings_account_path, notice: 'Verification email sent.'
1336
end

app/controllers/verifications_controller.rb

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,23 @@ def handle_add_alias(token)
8888
return redirect_to settings_account_path, alert: 'Email is linked to another account. Delete that account first to release it.'
8989
end
9090

91+
metadata = JSON.parse(token.metadata || '{}') rescue {}
92+
name = metadata['name'].presence
93+
9194
aliases = Alias.by_email(email)
9295
if aliases.exists?
96+
# Associate all existing aliases with this email
9397
aliases.find_each do |al|
9498
person.attach_alias!(al, user: user)
9599
al.update_columns(verified_at: Time.current)
96100
end
101+
# If a name was provided and no existing alias has that name, create a new alias
102+
if name.present? && !aliases.where(name: name).exists?
103+
Alias.create!(person: person, user: user, name: name, email: email, verified_at: Time.current)
104+
end
97105
else
98-
al = Alias.create!(person: person, user: user, name: email, email: email, verified_at: Time.current)
106+
# No existing aliases - create new one with provided name
107+
al = Alias.create!(person: person, user: user, name: name, email: email, verified_at: Time.current)
99108
person.update!(default_alias_id: al.id) if person.default_alias_id.nil?
100109
end
101110

app/views/settings/accounts/show.html.slim

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,18 @@
2929
p No connected accounts yet.
3030

3131
.settings-section
32-
h2 Email addresses
32+
h2 Add alias
3333
= form_with url: settings_emails_path, method: :post, local: true, class: "email-form", data: { turbo: false } do |f|
3434
.form-group
35-
= f.label :email, "Add email"
35+
= f.label :name, "Display name"
36+
= f.text_field :name, placeholder: "Your Name"
37+
.form-group
38+
= f.label :email, "Email address"
3639
= f.email_field :email, required: true, placeholder: "user@example.com"
37-
= f.submit "Send verification", class: "button-primary"
40+
= f.submit "Add email", class: "button-primary"
3841

3942
- if @aliases.any?
43+
h2 Aliases
4044
table.email-table
4145
thead
4246
tr

spec/requests/emails_management_spec.rb

Lines changed: 99 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ def attach_verified_alias(user, email:, primary: true)
2626
sign_in(email: 'me@example.com')
2727

2828
perform_enqueued_jobs do
29-
post settings_emails_path, params: { email: 'new-address@example.com' }
29+
post settings_emails_path, params: { email: 'new-address@example.com', name: 'My New Name' }
3030
expect(response).to redirect_to(settings_account_path)
3131
end
3232

@@ -38,7 +38,9 @@ def attach_verified_alias(user, email:, primary: true)
3838
get verification_path(token: raw)
3939
expect(response).to redirect_to(settings_account_path)
4040

41-
expect(Alias.by_email('new-address@example.com').where(user_id: user.id)).to exist
41+
new_alias = Alias.by_email('new-address@example.com').where(user_id: user.id).first
42+
expect(new_alias).to be_present
43+
expect(new_alias.name).to eq('My New Name')
4244

4345
post session_path, params: { email: 'new-address@example.com', password: 'secret' }
4446
expect(response).to redirect_to(root_path)
@@ -104,6 +106,101 @@ def attach_verified_alias(user, email:, primary: true)
104106
token&.destroy
105107
end
106108

109+
it 'requires name when adding a completely new email' do
110+
user = create(:user, password: 'secret', password_confirmation: 'secret')
111+
attach_verified_alias(user, email: 'me@example.com')
112+
113+
sign_in(email: 'me@example.com')
114+
115+
expect {
116+
post settings_emails_path, params: { email: 'brand-new@example.com' }
117+
}.not_to change { UserToken.count }
118+
119+
expect(response).to redirect_to(settings_account_path)
120+
expect(flash[:alert]).to match(/provide a display name/)
121+
end
122+
123+
it 'requires name when email is already verified by user' do
124+
user = create(:user, password: 'secret', password_confirmation: 'secret')
125+
attach_verified_alias(user, email: 'me@example.com')
126+
127+
sign_in(email: 'me@example.com')
128+
129+
expect {
130+
post settings_emails_path, params: { email: 'me@example.com' }
131+
}.not_to change { UserToken.count }
132+
133+
expect(response).to redirect_to(settings_account_path)
134+
expect(flash[:alert]).to match(/already verified/)
135+
end
136+
137+
it 'creates alias directly without verification when email is already verified by user' do
138+
user = create(:user, password: 'secret', password_confirmation: 'secret')
139+
attach_verified_alias(user, email: 'me@example.com')
140+
141+
sign_in(email: 'me@example.com')
142+
143+
expect {
144+
post settings_emails_path, params: { email: 'me@example.com', name: 'Alternative Name' }
145+
}.to change { Alias.by_email('me@example.com').count }.by(1)
146+
147+
expect(UserToken.count).to eq(0) # No verification token created
148+
149+
expect(response).to redirect_to(settings_account_path)
150+
expect(flash[:notice]).to eq('Alias added.')
151+
152+
new_alias = Alias.by_email('me@example.com').find_by(name: 'Alternative Name')
153+
expect(new_alias).to be_present
154+
expect(new_alias.user_id).to eq(user.id)
155+
expect(new_alias.verified_at).to be_present
156+
end
157+
158+
it 'creates additional alias with new name when associating existing aliases' do
159+
user = create(:user, password: 'secret', password_confirmation: 'secret')
160+
attach_verified_alias(user, email: 'me@example.com')
161+
162+
# Legacy alias for another email
163+
create(:alias, email: 'legacy@example.com', name: 'Old Name')
164+
165+
sign_in(email: 'me@example.com')
166+
167+
perform_enqueued_jobs do
168+
post settings_emails_path, params: { email: 'legacy@example.com', name: 'New Name' }
169+
expect(response).to redirect_to(settings_account_path)
170+
end
171+
172+
raw = extract_raw_token_from_mailer
173+
get verification_path(token: raw)
174+
expect(response).to redirect_to(settings_account_path)
175+
176+
aliases = Alias.by_email('legacy@example.com').where(user_id: user.id)
177+
expect(aliases.count).to eq(2)
178+
expect(aliases.pluck(:name)).to contain_exactly('Old Name', 'New Name')
179+
end
180+
181+
it 'does not create duplicate alias when name matches existing' do
182+
user = create(:user, password: 'secret', password_confirmation: 'secret')
183+
attach_verified_alias(user, email: 'me@example.com')
184+
185+
# Legacy alias for another email
186+
create(:alias, email: 'legacy2@example.com', name: 'Existing Name')
187+
188+
sign_in(email: 'me@example.com')
189+
190+
perform_enqueued_jobs do
191+
post settings_emails_path, params: { email: 'legacy2@example.com', name: 'Existing Name' }
192+
expect(response).to redirect_to(settings_account_path)
193+
end
194+
195+
raw = extract_raw_token_from_mailer
196+
get verification_path(token: raw)
197+
expect(response).to redirect_to(settings_account_path)
198+
199+
aliases = Alias.by_email('legacy2@example.com').where(user_id: user.id)
200+
expect(aliases.count).to eq(1)
201+
expect(aliases.first.name).to eq('Existing Name')
202+
end
203+
107204
def extract_raw_token_from_mailer
108205
mail = ActionMailer::Base.deliveries.last
109206
expect(mail).to be_present

0 commit comments

Comments
 (0)