Skip to content

Commit 9c92e7f

Browse files
committed
pause_2017: use email header objects when possible
I don't know this code well, so this may be a bit of a wreck. First, `$mgr->prepare_sendto` is updated. 1. prefer fullname to asciiname, because it is more polite 2. use real email header generation, not just concat 3. if we had "Bob <x@y>" and "Alice <x@y>" we now only send one email, not one per unique phrase; I think this is fine Where the prepare_sendto code was basically duplicated, I have switched things to use prepare_sendto. Internal report emails now go to the internal reporting address, not the public contact address. In some places, the code was quite tough for me to read, so I just left `$email` and the like used. This should work fine. It would be nice to add phases later. **I didn't understand this**: `$pause->{send_to}`. That was being set to a string, but I don't know really why or how it was used, although I saw some hints. The new code would've put ARRAY(0x1234) in that, so I changed the text. I will seek advice.
1 parent 1ceabbe commit 9c92e7f

File tree

7 files changed

+39
-38
lines changed

7 files changed

+39
-38
lines changed

lib/pause_2017/PAUSE/Web/Context.pm

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -132,26 +132,39 @@ sub fetchrow {
132132
### Mailer
133133

134134
sub prepare_sendto {
135-
my ($self, $active_user, $pause_user, @admin) = @_;
135+
my ($self, $active_user, $pause_user, $include_admin) = @_;
136136

137+
# %umailset is just used to uniq mail targets. Keys are email addresses we
138+
# will send to. The values are the names. If we end up seeing two entries
139+
# for one address, it will only be emailed once. This is acceptable.
140+
# -- rjbs, 2024-05-03
137141
my %umailset;
138-
my $name = $active_user->{asciiname} || $active_user->{fullname} || "";
139-
my $Uname = $pause_user->{asciiname} || $pause_user->{fullname} || "";
142+
my $name = $active_user->{fullname} || $active_user->{asciiname} || "";
143+
my $Uname = $pause_user->{fullname} || $pause_user->{asciiname} || "";
140144
if ($active_user->{secretemail}) {
141-
$umailset{qq{"$name" <$active_user->{secretemail}>}} = 1;
145+
$umailset{ $active_user->{secretemail} } = $name;
142146
} elsif ($active_user->{email}) {
143-
$umailset{qq{"$name" <$active_user->{email}>}} = 1;
147+
$umailset{ $active_user->{email} } = $name;
144148
}
145149
if ($active_user->{userid} ne $pause_user->{userid}) {
146150
if ($pause_user->{secretemail}) {
147-
$umailset{qq{"$Uname" <$pause_user->{secretemail}>}} = 1;
148-
}elsif ($pause_user->{email}) {
149-
$umailset{qq{"$Uname" <$pause_user->{email}>}} = 1;
151+
$umailset{ $pause_user->{secretemail} } = $Uname;
152+
} elsif ($pause_user->{email}) {
153+
$umailset{ $pause_user->{email} } = $Uname;
150154
}
151155
}
152-
my @to = keys %umailset;
153-
push @to, @admin if @admin;
154-
@to;
156+
157+
my @to;
158+
for my $addr (sort keys %umailset) {
159+
my $addr = Email::Address::XS->new($umailset{$addr}, $addr);
160+
push @to, PAUSE::Email->email_header_object_for_addresses($addr);
161+
}
162+
163+
if ($include_admin) {
164+
push @to, PAUSE::Email->report_email_header_object;
165+
}
166+
167+
return @to;
155168
}
156169

157170
sub send_mail_multi {

lib/pause_2017/PAUSE/Web/Controller/Admin.pm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ sub edit_ml {
156156
if ($saw_a_change) {
157157
$pause->{changed} = 1;
158158
my $mailblurb = $c->render_to_string("email/admin/edit_ml", format => "email");
159-
my @to = ($u->{secretemail}||$u->{email}, $mgr->config->mailto_admins);
159+
my @to = ($u->{secretemail}||$u->{email}, PAUSE::Email->report_email_header_object);
160160
warn "sending to[@to]";
161161
warn "mailblurb[$mailblurb]";
162162
my $header = {

lib/pause_2017/PAUSE/Web/Controller/Public/RequestId.pm

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,10 @@ sub request {
141141
}
142142
}
143143

144-
my @to = $mgr->config->mailto_admins;
144+
my @to = PAUSE::Email->report_email_header_object;
145145
push @to, $email;
146-
$pause->{send_to} = "@to";
146+
$pause->{send_to} = "$email"; # I don't understand what this is for XXX -- rjbs, 2024-05-03
147+
147148
my $time = time;
148149
if ($rationale) {
149150
# wrap it

lib/pause_2017/PAUSE/Web/Controller/User.pm

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ sub edit_uris {
109109
$pause->{changed} = 1;
110110

111111
my $mailbody = $c->render_to_string("email/user/edit_uris", format => "email");
112-
my @to = $mgr->prepare_sendto($u, $pause->{User}, $mgr->config->mailto_admins);
112+
my @to = $mgr->prepare_sendto($u, $pause->{User}, 1);
113113
my $header = {
114114
Subject => "Uri update for $selectedrec->{uriid}"
115115
};
@@ -204,7 +204,7 @@ sub reindex {
204204
$pause->{blurb} = $blurb;
205205
$pause->{eta} = $eta;
206206

207-
my @to = $mgr->prepare_sendto($u, $pause->{User}, $PAUSE::Config->{INTERNAL_REPORT_ADDRESS});
207+
my @to = $mgr->prepare_sendto($u, $pause->{User}, 1);
208208
my $mailbody = $c->render_to_string("email/user/reindex", format => "email");
209209
my $header = {
210210
Subject => "Scheduled for reindexing $u->{userid}"
@@ -274,7 +274,7 @@ sub reset_version {
274274
if ($blurb) {
275275
$pause->{blurb} = $blurb;
276276

277-
my @to = $mgr->prepare_sendto($u, $pause->{User}, $PAUSE::Config->{INTERNAL_REPORT_ADDRESS});
277+
my @to = $mgr->prepare_sendto($u, $pause->{User}, 1);
278278
my $mailbody = $c->render_to_string("email/user/reset_version", format => "email");
279279
my $header = {
280280
Subject => "Version reset for $u->{userid}"

lib/pause_2017/PAUSE/Web/Controller/User/Cred.pm

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,20 +190,22 @@ sub edit {
190190
if ($nu->{userid} && $nu->{userid} eq $pause->{User}{userid}) {
191191
$pause->{User} = $nu;
192192
}
193+
193194
# Send separate emails to user and public places because
194195
# CC leaks secretemail to others
195196
my @to;
196197
my %umailset;
197198
for my $lu ($u, $nu) {
198199
for my $att (qw(secretemail email)) {
199200
if ($lu->{$att}){
200-
$umailset{qq{<$lu->{$att}>}} = 1;
201+
$umailset{ $lu->{$att} } = 1;
201202
last;
202203
}
203204
}
204205
}
205-
push @to, join ", ", keys %umailset;
206-
push @to, $mgr->config->mailto_admins if $mailto_admins;
206+
push @to, sort keys %umailset;
207+
push @to, PAUSE::Email->report_email_header_object if $mailto_admins;
208+
207209
my $header = {Subject => "User update for $u->{userid}"};
208210
$mgr->send_mail_multi(\@to,$header, $mailblurb);
209211
} else {

lib/pause_2017/PAUSE/Web/Controller/User/Files.pm

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -121,23 +121,8 @@ sub delete {
121121
$pause->{blurb} = $blurb;
122122
$blurb = $c->render_to_string("email/user/delete_files", format => "email");
123123

124-
my %umailset;
125-
my $name = $u->{asciiname} || $u->{fullname} || "";
126-
my $Uname = $pause->{User}{asciiname} || $pause->{User}{fullname} || "";
127-
if ($u->{secretemail}) {
128-
$umailset{qq{"$name" <$u->{secretemail}>}} = 1;
129-
} elsif ($u->{email}) {
130-
$umailset{qq{"$name" <$u->{email}>}} = 1;
131-
}
132-
if ($u->{userid} ne $pause->{User}{userid}) {
133-
if ($pause->{User}{secretemail}) {
134-
$umailset{qq{"$Uname" <$pause->{User}{secretemail}>}} = 1;
135-
}elsif ($pause->{User}{email}) {
136-
$umailset{qq{"$Uname" <$pause->{User}{email}>}} = 1;
137-
}
138-
}
139-
$umailset{$PAUSE::Config->{INTERNAL_REPORT_ADDRESS}} = 1;
140-
my @to = keys %umailset;
124+
my @to = $mgr->prepare_sendto($u, $pause->{User}, 1);
125+
141126
my $header = {
142127
Subject => "Files of $u->{userid} scheduled for deletion"
143128
};

lib/pause_2017/PAUSE/Web/Plugin/UserRegistration.pm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ sub _send_otp_email {
9292
};
9393
my $header_str = join "\n", map {"$_: $header->{$_}"} keys %$header;
9494
warn "header[$header_str]otpwblurb[$otpwblurb]";
95-
$mgr->send_mail_multi( [ $email, $PAUSE::Config->{INTERNAL_REPORT_ADDRESS} ], $header, $otpwblurb);
95+
$mgr->send_mail_multi( [ $email, PAUSE::Email->report_email_header_object ], $header, $otpwblurb);
9696
}
9797

9898
sub _send_welcome_email {

0 commit comments

Comments
 (0)