Skip to content

Commit 385ae39

Browse files
committed
Switch the feedback emails to HTML.
The email content is generated by the `templates/ContentGenerator/Feedback/feedback_email.html.ep` template. All of the same content is there, just arranged nicely into tables and such. The link to the page the user sent the email from is of course an `a href`. For now it reads "To visit the page from which (user name) sent feedback, click here." where "click here" is the link. Note that it used to read "To visit the page from which the user sent feedback, long link." We have the user name so why not use it here? Also, when the sent message is displayed for the user in the browser, the message is formated much the same as it now is in the email. That is it is not wrapped and placed in a `pre` tag. Instead it is in a bordered `div` formatted with essentially the same style as in the email.
1 parent 28b21ed commit 385ae39

File tree

3 files changed

+206
-126
lines changed

3 files changed

+206
-126
lines changed

lib/WeBWorK/ContentGenerator/Feedback.pm

Lines changed: 11 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,9 @@ WeBWorK::ContentGenerator::Feedback - Send mail to professors.
2525
use Data::Dumper;
2626
use Email::Stuffer;
2727
use Try::Tiny;
28-
use Text::Wrap qw(wrap);
2928

3029
use WeBWorK::Upload;
31-
use WeBWorK::Utils qw(decodeAnswers createEmailSenderTransportSMTP fetchEmailRecipients);
30+
use WeBWorK::Utils qw(createEmailSenderTransportSMTP fetchEmailRecipients);
3231

3332
# request paramaters used
3433
#
@@ -137,52 +136,18 @@ sub initialize ($c) {
137136
my $subject = $ce->{mail}{feedbackSubjectFormat} || 'WeBWorK question from %c: %u set %s/prob %p';
138137
$subject =~ s/%([$chars])/defined $subject_map{$1} ? $subject_map{$1} : ''/eg;
139138

140-
# Get info about remote user.
141139
my $remote_host = $c->tx->remote_address || 'UNKNOWN';
142-
my $remote_port = $c->tx->remote_port || 'UNKNOWN';
143140

144-
my $systemURL = $c->url_for('root')->to_abs;
145-
146-
my $msg = sprintf("Message from %s (%s) via WeBWorK at\n%s\n\n", $user->full_name, $user->user_id, $systemURL);
147-
$msg .= "To visit the page from which the user sent feedback, go to:\n$emailableURL\n\n";
148-
149-
if ($feedback) {
150-
$msg .= sprintf("%s (%s) wrote:\n\n\n%s\n\n\n", $user->full_name, $user->user_id, $feedback);
151-
}
152-
if ($problem and $verbosity >= 1) {
153-
$msg .=
154-
qq/***** Data about the problem processor: ***** \n\n/
155-
. 'Display Mode: '
156-
. $c->param('displayMode') . "\n"
157-
. 'Show Old Answers: '
158-
. ($c->param('showOldAnswers') ? 'yes' : 'no') . "\n"
159-
. 'Show Correct Answers: '
160-
. ($c->param('showCorrectAnswers') ? 'yes' : 'no') . "\n"
161-
. 'Show Hints: '
162-
. ($c->param('showHints') ? 'yes' : 'no') . "\n"
163-
. 'Show Solutions: '
164-
. ($c->param('showSolutions') ? 'yes' : 'no') . "\n\n";
165-
}
166-
167-
if ($user && $verbosity >= 1) {
168-
$msg .= "***** Data about the user: *****\n\n";
169-
$msg .= $c->format_user($user) . "\n";
170-
$msg .= "$remote_host:$remote_port\n";
171-
}
172-
173-
if ($problem && $verbosity >= 1) {
174-
$msg .= "***** Data about the problem: *****\n\n";
175-
$msg .= $c->format_userproblem($problem) . "\n";
176-
}
177-
if ($set && $verbosity >= 1) {
178-
$msg .= "***** Data about the homework set: *****\n\n" . $c->format_userset($set) . "\n";
179-
}
180-
if ($ce && $verbosity >= 2) {
181-
$msg .= "***** Data about the environment: *****\n\n" . Dumper($ce) . "\n\n";
182-
}
183-
184-
my $email = Email::Stuffer->to(join(',', @recipients))->subject($subject)->text_body($msg)
185-
->header('X-Remote-Host' => $remote_host);
141+
my $email = Email::Stuffer->to(join(',', @recipients))->subject($subject)->html_body($c->render_to_string(
142+
'ContentGenerator/Feedback/feedback_email',
143+
user => $user,
144+
emailableURL => $emailableURL,
145+
feedback => $feedback,
146+
problem => $problem,
147+
set => $set,
148+
verbosity => $verbosity,
149+
remote_host => $remote_host,
150+
))->header('X-Remote-Host' => $remote_host);
186151
if ($ce->{feedback_sender_email}) {
187152
my $from_name = $user ? $user->full_name : $ce->{generic_sender_name};
188153
$email->from("$from_name <$ce->{feedback_sender_email}>")->reply_to($sender);
@@ -263,81 +228,4 @@ sub page_title ($c) {
263228
return $c->ce->{feedback_button_name} || $c->maketext('E-mail Instructor');
264229
}
265230

266-
sub format_user ($c, $user) {
267-
my $ce = $c->ce;
268-
269-
my $result = "User ID: " . $user->user_id . "\n";
270-
$result .= "Name: " . $user->full_name . "\n";
271-
$result .= "Email: " . $user->email_address . "\n";
272-
unless ($ce->{blockStudentIDinFeedback}) {
273-
$result .= "Student ID: " . $user->student_id . "\n";
274-
}
275-
276-
my $status_name = $ce->status_abbrev_to_name($user->status);
277-
my $status_string =
278-
defined $status_name
279-
? "$status_name ('" . $user->status . "')"
280-
: $user->status . " (unknown status abbreviation)";
281-
$result .= "Status: $status_string\n";
282-
283-
$result .= "Section: " . $user->section . "\n";
284-
$result .= "Recitation: " . $user->recitation . "\n";
285-
$result .= "Comment: " . $user->comment . "\n";
286-
287-
return $result;
288-
}
289-
290-
sub format_userset ($c, $set) {
291-
my $ce = $c->ce;
292-
293-
my $result = "Set ID: " . $set->set_id . "\n";
294-
$result .= "Set header file: " . $set->set_header . "\n";
295-
$result .= "Hardcopy header file: " . $set->hardcopy_header . "\n";
296-
297-
$result .= "Open date: " . $c->formatDateTime($set->open_date) . "\n";
298-
$result .= "Due date: " . $c->formatDateTime($set->due_date) . "\n";
299-
$result .= "Answer date: " . $c->formatDateTime($set->answer_date) . "\n";
300-
$result .= "Visible: " . ($set->visible ? "yes" : "no") . "\n";
301-
$result .= "Assignment type: " . $set->assignment_type . "\n";
302-
if ($set->assignment_type =~ /gateway/) {
303-
$result .= "Attempts per version: " . $set->assignment_type . "\n";
304-
$result .= "Time interval: " . $set->time_interval . "\n";
305-
$result .= "Versions per interval: " . $set->versions_per_interval . "\n";
306-
$result .= "Version time limit: " . $set->version_time_limit . "\n";
307-
$result .= "Version creation time: " . $c->formatDateTime($set->version_creation_time) . "\n";
308-
$result .= "Problem randorder: " . $set->problem_randorder . "\n";
309-
$result .= "Version last attempt time: " . $set->version_last_attempt_time . "\n";
310-
}
311-
312-
return $result;
313-
}
314-
315-
sub format_userproblem ($c, $problem) {
316-
my $ce = $c->ce;
317-
318-
my $result = "Problem ID: " . $problem->problem_id . "\n";
319-
$result .= "Source file: " . $problem->source_file . "\n";
320-
$result .= "Value: " . $problem->value . "\n";
321-
$result .=
322-
"Max attempts " . ($problem->max_attempts == -1 ? "unlimited" : $problem->max_attempts) . "\n";
323-
$result .= "Random seed: " . $problem->problem_seed . "\n";
324-
$result .= "Status: " . $problem->status . "\n";
325-
$result .= "Attempted: " . ($problem->attempted ? "yes" : "no") . "\n";
326-
327-
my %last_answer = decodeAnswers($problem->last_answer);
328-
if (%last_answer) {
329-
$result .= "Last answer:\n";
330-
foreach my $key (sort keys %last_answer) {
331-
$result .= "\t$key: $last_answer{$key}\n" if $last_answer{$key};
332-
}
333-
} else {
334-
$result .= "Last answer: none\n";
335-
}
336-
337-
$result .= "Number of correct attempts: " . $problem->num_correct . "\n";
338-
$result .= "Number of incorrect attempts: " . $problem->num_incorrect . "\n";
339-
340-
return $result;
341-
}
342-
343231
1;

templates/ContentGenerator/Feedback.html.ep

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
% use Text::Wrap qw(wrap);
2-
%
31
% unless ($authz->hasPermissions(param('user'), 'submit_feedback')) {
42
<p><%= maketext('You are not allowed to send email.') %></p>
53
<p><%= link_to maketext('Cancel E-Mail') => $returnURL %></p>
@@ -15,7 +13,15 @@
1513
% if (defined param('sendFeedback') && !stash('send_error')) {
1614
<p><%= maketext('Your message was sent successfully.') %></p>
1715
<p><%= link_to maketext('Return to your work') => $returnURL =%></p>
18-
<pre><%= wrap('', '', param('feedback')) =%></pre>
16+
<div class="border border-dark rounded mb-1 p-3">
17+
% for (split /\n\r?/, param('feedback')) {
18+
% if ($_) {
19+
<p class="m-0" style="white-space: pre-wrap;"><%= $_ %></p>
20+
% } else {
21+
<div class="mt-3"></div>
22+
% }
23+
% }
24+
</div>
1925
% } else {
2026
<%= form_for current_route, method => 'POST', enctype => 'multipart/form-data', begin =%>
2127
<%= $c->hidden_authen_fields =%>
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
% use WeBWorK::Utils qw(decodeAnswers);
2+
%
3+
<html>
4+
<head>
5+
<%= stylesheet begin =%>
6+
.data-table td, .data-table th {
7+
text-align: left;
8+
padding: 0.25rem;
9+
}
10+
.data-table.bordered {
11+
border-collapse: collapse;
12+
}
13+
.data-table.bordered > * > tr > td, .data-table.bordered > * > tr > th {
14+
border: 1px solid black;
15+
}
16+
.data-table.bordered thead tr:first-child th {
17+
border-bottom-width: 2px;
18+
}
19+
.mb-1 {
20+
margin-bottom: 1rem
21+
}
22+
.user-message {
23+
border: 1px solid black;
24+
border-radius: 0.375rem;
25+
padding: 1rem;
26+
}
27+
.user-message-line {
28+
margin: 0;
29+
white-space: pre-wrap;
30+
}
31+
<% end =%>
32+
</head>
33+
<body>
34+
<p>
35+
Message from <%= $user->full_name %> (<%= $user->user_id %>) via WeBWorK at <%= url_for('root')->to_abs %>
36+
</p>
37+
<p>
38+
To visit the page from which <%= $user->first_name %> sent feedback,
39+
<%= link_to 'click here' => $emailableURL %>.
40+
</p>
41+
% if ($feedback) {
42+
<p><%= $user->full_name %> (<%= $user->user_id %>) wrote:</p>
43+
<div class="user-message mb-1">
44+
% for (split /\n\r?/, $feedback) {
45+
% if ($_) {
46+
<p class="user-message-line"><%= $_ %></p>
47+
% } else {
48+
<div style="margin-top: 1em"></div>
49+
% }
50+
% }
51+
</div>
52+
% }
53+
% if ($problem && $verbosity >= 1) {
54+
<table class="data-table bordered mb-1">
55+
<thead>
56+
<tr><th colspan="2">Data about the problem processor</th></tr>
57+
</thead>
58+
<tbody>
59+
<tr>
60+
<th>Display Mode:</th>
61+
<td><%= param('displayMode') %></td>
62+
</tr>
63+
<tr>
64+
<th>Show Old Answers:</th>
65+
<td><%= param('showOldAnswers') ? 'yes' : 'no' %></td>
66+
</tr>
67+
<tr>
68+
<th>Show Correct Answers:</th>
69+
<td><%= param('showCorrectAnswers') ? 'yes' : 'no' %></td>
70+
</tr>
71+
<tr>
72+
<th>Show Hints:</th>
73+
<td><%= param('showHints') ? 'yes' : 'no' %></td>
74+
</tr>
75+
<tr>
76+
<th>Show Solutions:</th>
77+
<td><%= param('showSolutions') ? 'yes' : 'no' %></td>
78+
</tr>
79+
</tbody>
80+
</table>
81+
% }
82+
%
83+
% if ($user && $verbosity >= 1) {
84+
<table class="data-table bordered mb-1">
85+
<thead>
86+
<tr><th colspan="2">Data about the user</th></tr>
87+
</thead>
88+
<tbody>
89+
<tr><th>User ID:</th><td><%= $user->user_id %></td></tr>
90+
<tr><th>Name:</th><td><%= $user->full_name %></td></tr>
91+
<tr><th>Email:</th><td><%= $user->email_address %></td></tr>
92+
% unless ($ce->{blockStudentIDinFeedback}) {
93+
<tr><th>Student ID:</th><td><%= $user->student_id %></td></tr>
94+
% }
95+
% my $status_name = $ce->status_abbrev_to_name($user->status);
96+
%my $status_string =
97+
% defined $status_name
98+
% ? "$status_name ('" . $user->status . q{')}
99+
% : $user->status . ' (unknown status abbreviation)';
100+
<tr><th>Status:</th><td><%= $status_string %></td></tr>
101+
<tr><th>Section:</th><td><%= $user->section %></td></tr>
102+
<tr><th>Recitation:</th><td><%= $user->recitation %></td></tr>
103+
<tr><th>Comment:</th><td><%= $user->comment %></td></tr>
104+
<tr><th>User IP:</th><td><%= $remote_host %>:<%= $c->tx->remote_port || 'UNKNOWN' %></td></tr>
105+
</tbody>
106+
</table>
107+
% }
108+
% if ($problem && $verbosity >= 1) {
109+
<table class="data-table bordered mb-1">
110+
<thead>
111+
<tr><th colspan="2">Data about the problem</th></tr>
112+
</thead>
113+
<tbody>
114+
<tr><th>Problem ID:</th><td><%= $problem->problem_id %></td></tr>
115+
<tr><th>Source file:</th><td><%= $problem->source_file %></td></tr>
116+
<tr><th>Value:</th><td><%= $problem->value %></td></tr>
117+
<tr>
118+
<th>Max attempts</th>
119+
<td><%= $problem->max_attempts == -1 ? 'unlimited' : $problem->max_attempts %></td>
120+
</tr>
121+
<tr><th>Random seed:</th><td><%= $problem->problem_seed %></td></tr>
122+
<tr><th>Status:</th><td><%= $problem->status %></td></tr>
123+
<tr><th>Attempted:</th><td><%= $problem->attempted ? 'yes' : 'no' %></td></tr>
124+
% my %last_answer = decodeAnswers($problem->last_answer);
125+
<tr>
126+
<th>Last answer:</th>
127+
% if (%last_answer) {
128+
<td>
129+
<table class="data-table">
130+
% for my $key (sort keys %last_answer) {
131+
% if ($last_answer{$key}) {
132+
<tr><td><%= $key %>:</td><td><%= $last_answer{$key} %></td></tr>
133+
% }
134+
% }
135+
</table>
136+
</td>
137+
% } else {
138+
<td>none</td>
139+
% }
140+
</tr>
141+
<tr><th>Number of correct attempts:</th><td><%= $problem->num_correct %></td></tr>
142+
<tr><th>Number of incorrect attempts:</th><td><%= $problem->num_incorrect %></td></tr>
143+
</tbody>
144+
</table>
145+
% }
146+
% if ($set && $verbosity >= 1) {
147+
<table class="data-table bordered mb-1">
148+
<thead>
149+
<tr><th colspan="2">Data about the homework set</th></tr>
150+
</thead>
151+
<tbody>
152+
<tr><th>Set ID:</th><td><%= $set->set_id %></td></tr>
153+
<tr><th>Set header file:</th><td><%= $set->set_header %></td></tr>
154+
<tr><th>Hardcopy header file:</th><td><%= $set->hardcopy_header %></td></tr>
155+
<tr><th>Open date:</th><td><%= $c->formatDateTime($set->open_date) %></td></tr>
156+
<tr><th>Due date:</th><td><%= $c->formatDateTime($set->due_date) %></td></tr>
157+
<tr><th>Answer date:</th><td><%= $c->formatDateTime($set->answer_date) %></td></tr>
158+
<tr><th>Visible:</th><td><%= $set->visible ? 'yes' : 'no' %></td></tr>
159+
<tr><th>Assignment type:</th><td><%= $set->assignment_type %></td></tr>
160+
% if ($set->assignment_type =~ /gateway/) {
161+
<tr><th>Attempts per version:</th><td><%= $set->assignment_type %></td></tr>
162+
<tr><th>Time interval:</th><td><%= $set->time_interval %></td></tr>
163+
<tr><th>Versions per interval:</th><td><%= $set->versions_per_interval %></td></tr>
164+
<tr><th>Version time limit:</th><td><%= $set->version_time_limit %></td></tr>
165+
<tr>
166+
<th>Version creation time:</th>
167+
<td><%= $c->formatDateTime($set->version_creation_time) %></td>
168+
</tr>
169+
<tr><th>Problem randorder:</th><td><%= $set->problem_randorder %></td></tr>
170+
<tr><th>Version last attempt time:</th><td><%= $set->version_last_attempt_time %></td></tr>
171+
% }
172+
</tbody>
173+
</table>
174+
% }
175+
% if ($verbosity >= 2) {
176+
<table class="data-table bordered">
177+
<thead>
178+
<tr><th colspan="2">Data about the environment</th></tr>
179+
</thead>
180+
<tbody>
181+
<tr><td><pre><%= dumper($ce) %></pre></td></tr>
182+
</tbody>
183+
</table>
184+
% }
185+
</body>
186+
</html>

0 commit comments

Comments
 (0)