Skip to content

Commit b264ca6

Browse files
committed
Improve alias deletion
Now we display message count for each alias, and we only let users remove aliases that have no messages associated with them.
1 parent a2a2f3f commit b264ca6

4 files changed

Lines changed: 86 additions & 6 deletions

File tree

app/controllers/settings/accounts_controller.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ def show
99
) || []
1010
@identities = current_user.identities.order(:provider, :email, :uid)
1111
@default_alias_id = current_user.person&.default_alias_id
12+
13+
# Preload mention counts (CC/TO) for all aliases
14+
alias_ids = @aliases.map(&:id)
15+
@mention_counts = alias_ids.any? ? Mention.where(alias_id: alias_ids).group(:alias_id).count : {}
1216
end
1317

1418
private

app/controllers/settings/emails_controller.rb

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,18 @@ def create
3737

3838
def destroy
3939
al = current_user.person.aliases.find(params[:id])
40+
4041
if current_user.person&.default_alias_id == al.id
41-
redirect_to settings_account_path, alert: 'Cannot remove primary alias.'
42-
else
43-
new_person = Person.create!(default_alias_id: al.id)
44-
al.update!(user_id: nil, verified_at: nil, person_id: new_person.id)
45-
redirect_to settings_account_path, notice: 'Email removed.'
42+
return redirect_to settings_account_path, alert: 'Cannot remove primary alias.'
43+
end
44+
45+
mention_count = Mention.where(alias_id: al.id).count
46+
if al.sender_count > 0 || mention_count > 0
47+
return redirect_to settings_account_path, alert: 'Cannot remove alias with message history.'
4648
end
49+
50+
al.destroy!
51+
redirect_to settings_account_path, notice: 'Alias removed.'
4752
end
4853

4954
def primary

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
tr
4747
th Name
4848
th Email
49+
th Messages
4950
th Verified
5051
th Primary
5152
th Actions
@@ -57,12 +58,15 @@
5758
= al.email
5859
- if al.mention_only?
5960
span.alias-badge.mention-only (mention only)
61+
td.message-counts title="Sent / CC'd"
62+
= "#{al.sender_count} / #{@mention_counts[al.id] || 0}"
6063
td = al.verified_at ? "Yes" : "No"
6164
td = (al.id == @default_alias_id) ? "Yes" : "No"
6265
td
6366
- unless al.id == @default_alias_id
6467
= button_to "Make primary", primary_settings_email_path(al), method: :post, class: "button-secondary"
65-
- unless al.id == @default_alias_id
68+
- has_messages = al.sender_count > 0 || (@mention_counts[al.id] || 0) > 0
69+
- if al.id != @default_alias_id && !has_messages
6670
= button_to "Remove", settings_email_path(al), method: :delete, data: { turbo_confirm: "Remove this email?" }, class: "button-danger"
6771
- else
6872
p No emails yet.

spec/requests/emails_management_spec.rb

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,73 @@ def attach_verified_alias(user, email:, primary: true)
201201
expect(aliases.first.name).to eq('Existing Name')
202202
end
203203

204+
describe 'alias removal' do
205+
it 'allows removing alias with no messages' do
206+
user = create(:user, password: 'secret', password_confirmation: 'secret')
207+
primary = attach_verified_alias(user, email: 'me@example.com')
208+
secondary = create(:alias, user: user, person: user.person, email: 'other@example.com', name: 'Other', verified_at: Time.current, sender_count: 0)
209+
210+
sign_in(email: 'me@example.com')
211+
212+
expect {
213+
delete settings_email_path(secondary)
214+
}.to change { Alias.count }.by(-1)
215+
216+
expect(response).to redirect_to(settings_account_path)
217+
expect(flash[:notice]).to eq('Alias removed.')
218+
expect(Alias.find_by(id: secondary.id)).to be_nil
219+
end
220+
221+
it 'blocks removing alias with sent messages' do
222+
user = create(:user, password: 'secret', password_confirmation: 'secret')
223+
primary = attach_verified_alias(user, email: 'me@example.com')
224+
secondary = create(:alias, user: user, person: user.person, email: 'other@example.com', name: 'Other', verified_at: Time.current, sender_count: 5)
225+
226+
sign_in(email: 'me@example.com')
227+
228+
expect {
229+
delete settings_email_path(secondary)
230+
}.not_to change { user.person.aliases.count }
231+
232+
expect(response).to redirect_to(settings_account_path)
233+
expect(flash[:alert]).to match(/message history/)
234+
end
235+
236+
it 'blocks removing alias with CC mentions' do
237+
user = create(:user, password: 'secret', password_confirmation: 'secret')
238+
primary = attach_verified_alias(user, email: 'me@example.com')
239+
secondary = create(:alias, user: user, person: user.person, email: 'other@example.com', name: 'Other', verified_at: Time.current, sender_count: 0)
240+
241+
# Create a mention for this alias
242+
topic = create(:topic)
243+
message = create(:message, topic: topic)
244+
create(:mention, message: message, alias: secondary, person: user.person)
245+
246+
sign_in(email: 'me@example.com')
247+
248+
expect {
249+
delete settings_email_path(secondary)
250+
}.not_to change { user.person.aliases.count }
251+
252+
expect(response).to redirect_to(settings_account_path)
253+
expect(flash[:alert]).to match(/message history/)
254+
end
255+
256+
it 'blocks removing primary alias' do
257+
user = create(:user, password: 'secret', password_confirmation: 'secret')
258+
primary = attach_verified_alias(user, email: 'me@example.com')
259+
260+
sign_in(email: 'me@example.com')
261+
262+
expect {
263+
delete settings_email_path(primary)
264+
}.not_to change { user.person.aliases.count }
265+
266+
expect(response).to redirect_to(settings_account_path)
267+
expect(flash[:alert]).to match(/primary alias/)
268+
end
269+
end
270+
204271
def extract_raw_token_from_mailer
205272
mail = ActionMailer::Base.deliveries.last
206273
expect(mail).to be_present

0 commit comments

Comments
 (0)