Skip to content

Commit

Permalink
Merge 6d2795b into edec516
Browse files Browse the repository at this point in the history
  • Loading branch information
MorayMySoc authored Aug 16, 2022
2 parents edec516 + 6d2795b commit 297465d
Show file tree
Hide file tree
Showing 22 changed files with 287 additions and 55 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
- Add an index on problem(external_id) to speed up bin/fetch --updates
- Upgrade Net::DNS and libwww to deal with IPv6 issues.
- Add send_state column to updates. #3865
- Enable alternative response from templates to be emailed to issue reporter. #4001
- Security
- Permit control over database connection `sslmode` via $FMS_DB_SSLMODE
- Open311 improvements:
Expand Down
1 change: 1 addition & 0 deletions bin/update-schema
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ else {
# (assuming schema change files are never half-applied, which should be the case)
sub get_db_version {
return 'EMPTY' if ! table_exists('problem');
return '0079' if column_exists('response_templates', 'email_text');
return '0078' if column_exists('problem','send_fail_body_ids');
return '0077' if column_exists('comment', 'send_state');
return '0076' if index_exists('problem_external_id_idx');
Expand Down
4 changes: 4 additions & 0 deletions db/downgrade_0079---0078.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
begin;
alter table response_templates drop email_text;
alter table comment drop private_email_text;
commit;
4 changes: 3 additions & 1 deletion db/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,8 @@ create table comment (
send_fail_count integer not null default 0,
send_fail_reason text,
send_fail_timestamp timestamp,
whensent timestamp
whensent timestamp,
private_email_text text
);

create index comment_user_id_idx on comment(user_id);
Expand Down Expand Up @@ -532,6 +533,7 @@ create table response_templates (
body_id int references body(id) not null,
title text not null,
text text not null,
email_text text,
created timestamp not null default current_timestamp,
auto_response boolean NOT NULL DEFAULT 'f',
state text,
Expand Down
2 changes: 2 additions & 0 deletions db/schema_0079-add-response-template-email-text.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
alter table response_templates add email_text text;
alter table comment add private_email_text text;
6 changes: 6 additions & 0 deletions docs/_includes/admin-tasks-content.md
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,12 @@ will automatically be added as a first update to any new report created that
matches the template (ie. in the relevant category if assigned). This lets
you give e.g. estimated timescales or other useful information up front.

If you enter text in the ‘Text for email alert field’, the template text will update
the report on the website and the email text will be sent to the user if they have
opted into alerts.



#### Editing or deleting a template

Click on ‘Templates’ in the admin menu. You will see a table of existing templates. Click on ‘Edit’
Expand Down
5 changes: 5 additions & 0 deletions perllib/FixMyStreet/App/Controller/Admin/Templates.pm
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,12 @@ sub edit : Path : Args(2) {
$c->stash->{errors}->{title} = _("There is already a template with that title.");
}

if ($c->get_param('email') && !$c->get_param('text') ) {
$c->stash->{errors}->{email_text} = _("There must be template text if there is alternative email text.");
};
$template->text( $c->get_param('text') );
$template->email_text( $c->get_param('email') || '');

$template->state( $c->get_param('state') );

my $ext_code = $c->cobrand->call_hook('admin_templates_external_status_code_hook');
Expand Down
2 changes: 2 additions & 0 deletions perllib/FixMyStreet/DB/Result/Comment.pm
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ __PACKAGE__->add_columns(
{ data_type => "timestamp", is_nullable => 1 },
"send_state",
{ data_type => "text", default_value => "unprocessed", is_nullable => 0 },
"private_email_text",
{ data_type => "text", is_nullable => 1 },
);
__PACKAGE__->set_primary_key("id");
__PACKAGE__->has_many(
Expand Down
2 changes: 2 additions & 0 deletions perllib/FixMyStreet/DB/Result/ResponseTemplate.pm
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ __PACKAGE__->add_columns(
{ data_type => "text", is_nullable => 1 },
"external_status_code",
{ data_type => "text", is_nullable => 1 },
"email_text",
{ data_type => "text", is_nullable => 1 },
);
__PACKAGE__->set_primary_key("id");
__PACKAGE__->add_unique_constraint("response_templates_body_id_title_key", ["body_id", "title"]);
Expand Down
3 changes: 2 additions & 1 deletion perllib/FixMyStreet/Queue/Item/Report.pm
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ sub _add_confirmed_update {
);

my $template = $problem->response_template_for('confirmed', 'dummy', '', '');
my $description = $updates->comment_text_for_request($template, {}, $problem);
my ($description, $email_text) = $updates->comment_text_for_request($template, {}, $problem);
next unless $description;

my $request = {
Expand All @@ -343,6 +343,7 @@ sub _add_confirmed_update {
# which uses current_timestamp (and thus microseconds) whilst this update
# is rounded down to the nearest second
comment_time => DateTime->now->add( seconds => 1 ),
email_text => $email_text,
status => 'open',
description => $description,
};
Expand Down
30 changes: 25 additions & 5 deletions perllib/FixMyStreet/Script/Alerts.pm
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ sub send_alert_type {
$item_table.problem_state as item_problem_state,
$item_table.cobrand as item_cobrand,
$item_table.extra as item_extra,
$item_table.private_email_text as item_private_email_text,
$head_table.*
from alert, $item_table, $head_table
where alert.parameter::integer = $head_table.id
Expand Down Expand Up @@ -85,14 +86,13 @@ sub send_alert_type {
my $last_problem_state = 'confirmed';
my %data = ( template => $alert_type->template, data => [], schema => $schema );
while (my $row = $query->fetchrow_hashref) {

$row->{is_new_update} = defined($row->{item_text});

my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker($row->{alert_cobrand})->new();
$cobrand->set_lang_and_domain( $row->{alert_lang}, 1, FixMyStreet->path_to('locale')->stringify );

# Cobranded and non-cobranded messages can share a database. In this case, the conf file
# should specify a vhost to send the reports for each cobrand, so that they don't get sent
# Cobranded and non-cobranded messages can share a database. In this case, the conf file
# should specify a vhost to send the reports for each cobrand, so that they don't get sent
# more than once if there are multiple vhosts running off the same database. The email_host
# call checks if this is the host that sends mail for this cobrand.
next unless $cobrand->email_host;
Expand Down Expand Up @@ -156,7 +156,6 @@ sub send_alert_type {
} else {
_extra_new_area_data($row, $ref);
}

push @{$data{data}}, $row;

if (!$data{alert_user_id}) {
Expand Down Expand Up @@ -360,6 +359,28 @@ sub _send_aggregated_alert(%) {
} );
$data{unsubscribe_url} = $cobrand->base_url( $data{cobrand_data} ) . '/A/' . $token->token;

# Filter out alerts that have templated email responses for separate sending and send those to the problem reporter
my @template_data = grep { $_->{item_private_email_text } && $_->{user_id} == $_->{alert_user_id} } @{ $data{data} };
@{ $data{data} } = grep {! $_->{item_private_email_text } || $_->{user_id} != $_->{alert_user_id} } @{ $data{data} };

if (@template_data) {
my %template_data = %data;
$template_data{data} = [@template_data];
$template_data{template} = 'templated_email_alert-update';
trigger_alert_sending($alert_by, $token, %template_data);
};

if (@{ $data{data} }) {
trigger_alert_sending($alert_by, $token, %data);
}

}

sub trigger_alert_sending {
my $alert_by = shift;
my $token = shift;
my %data = @_;

my $result;
if ($alert_by eq 'phone') {
$result = _send_aggregated_alert_phone(%data);
Expand All @@ -380,7 +401,6 @@ sub _send_aggregated_alert_email {
my $cobrand = $data{cobrand};

FixMyStreet::Map::set_map_class($cobrand);

my $sender = FixMyStreet::Email::unique_verp_id([ 'alert', $data{alert_id} ], $cobrand->call_hook('verp_email_domain'));
my $result = FixMyStreet::Email::send_cron(
$data{schema},
Expand Down
10 changes: 6 additions & 4 deletions perllib/FixMyStreet/Template.pm
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,14 @@ it all to text.

sub email_sanitize_text : Fn('email_sanitize_text') {
my $update = shift;
my $column = shift;

my $text = $update->{item_text};
my $text = $column ? $update->{$column} : $update->{item_text};
my $extra = $update->{item_extra};
utf8::encode($extra) if $extra;
$extra = $extra ? RABX::wire_rd(new IO::String($extra)) : {};

my $staff = $extra->{is_superuser} || $extra->{is_body_user};
my $staff = $extra->{is_superuser} || $extra->{is_body_user} || $column;

return $text unless $staff;

Expand Down Expand Up @@ -250,13 +251,14 @@ in updates from staff/superusers.

sub email_sanitize_html : Fn('email_sanitize_html') {
my $update = shift;
my $column = shift;

my $text = $update->{item_text};
my $text = $column ? $update->{$column} : $update->{item_text};
my $extra = $update->{item_extra};
utf8::encode($extra) if $extra;
$extra = $extra ? RABX::wire_rd(new IO::String($extra)) : {};

my $staff = $extra->{is_superuser} || $extra->{is_body_user};
my $staff = $extra->{is_superuser} || $extra->{is_body_user} || $column;

return _staff_html_markup($text, $staff);
}
Expand Down
16 changes: 11 additions & 5 deletions perllib/Open311/UpdatesBase.pm
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ sub find_problem {

sub process_update {
my ($self, $request, $p) = @_;

my $open311 = $self->current_open311;
my $body = $self->current_body;

Expand All @@ -161,7 +162,10 @@ sub process_update {
my $template = $p->response_template_for(
$state, $old_state, $external_status_code, $old_external_status_code
);
my $text = $self->comment_text_for_request($template, $request, $p);
my ($text, $email_text) = $self->comment_text_for_request($template, $request, $p);
if (!$email_text && $request->{email_text}) {
$email_text = $request->{email_text};
};
my $comment = $self->schema->resultset('Comment')->new(
{
problem => $p,
Expand All @@ -171,6 +175,7 @@ sub process_update {
text => $text,
confirmed => $request->{comment_time},
created => $request->{comment_time},
private_email_text => $email_text,
}
);

Expand Down Expand Up @@ -265,22 +270,23 @@ sub process_update {
sub comment_text_for_request {
my ($self, $template, $request, $problem) = @_;

my $template_email_text = $template->email_text if $template;
$template = $template->text if $template;

my $desc = $request->{description} || '';
if ($desc && (!$template || ($template !~ /\{\{description}}/ && !$request->{prefer_template}))) {
return $desc;
return ($desc, undef);
}

if ($template) {
$template =~ s/\{\{description}}/$desc/;
return $template;
return ($template, $template_email_text);
}

return "" if $self->blank_updates_permitted;
return ("", undef) if $self->blank_updates_permitted;

print STDERR "Couldn't determine update text for $request->{update_id} (report " . $problem->id . ")\n";
return "";
return ("", undef);
}

1;
1 change: 0 additions & 1 deletion perllib/Utils.pm
Original file line number Diff line number Diff line change
Expand Up @@ -257,5 +257,4 @@ sub _part {
}
}


1;
21 changes: 21 additions & 0 deletions t/app/controller/admin/templates.t
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,27 @@ subtest "templates that set state and external_status_code can't be added" => su
is $oxfordshire->response_templates->count, 0, "Invalid response template wasn't added";
};

subtest "templates that set private email text but not public text can't be added" => sub {
$mech->delete_response_template($_) for $oxfordshire->response_templates;
$mech->log_in_ok( $superuser->email );
$mech->get_ok( "/admin/templates/" . $oxfordshire->id . "/new" );

my $fields = {
title => "Report marked fixed - all cats",
text => "",
email => "Thank you for your report. This problems has been fixed.",
auto_response => 'on',
state => 'fixed - council',
external_status_code => '100',
};
$mech->submit_form_ok( { with_fields => $fields } );
is $mech->uri->path, '/admin/templates/' . $oxfordshire->id . '/new', 'not redirected';
$mech->content_contains( 'Please correct the errors below' );
$mech->content_contains( 'There must be template text if there is alternative email text.' );

is $oxfordshire->response_templates->count, 0, "Invalid response template wasn't added";
};

subtest "category groups are shown" => sub {
FixMyStreet::override_config {
ALLOWED_COBRANDS => [ 'oxfordshire' ],
Expand Down
53 changes: 52 additions & 1 deletion t/app/controller/report_new_update.t
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,58 @@ subtest "test resending does not leave another initial auto-update" => sub {
$report->discard_changes;
$report->update({ whensent => undef });
FixMyStreet::Script::Reports::send(0, 0, 1);
is +FixMyStreet::DB->resultset('Comment')->count, 1;
my $comments = FixMyStreet::DB->resultset('Comment');
is $comments->count, 1;
$comments->delete;
$report->delete;
};

$template->update({ email_text => 'Thanks for your report.
This is the email <a href="https://google.com">alternative</a>.',
});

subtest "test report creation with initial auto-update and alternative email text" => sub {

my $report = make_report();
my $report_id = $report->id;
$mech->clear_emails_ok;

my $user3 = $mech->log_in_ok('[email protected]');
$mech->log_in_ok($user3->email);
$mech->get_ok("/report/$report_id");
$mech->submit_form_ok({ button => 'alert', with_fields => { type => 'updates' } });
$mech->log_out_ok;

FixMyStreet::Script::Alerts::send_updates();

my @emails = $mech->get_email;

is scalar @emails, 2, 'Two alerts sent';

my ($email_for_reporter) = grep { $_->header('To') =~ /test-2/ } @emails;
my ($email_for_subscriber) = grep { $_->header('To') =~ /test-3/ } @emails;

is $mech->get_text_body_from_email($email_for_subscriber) =~ /Thanks for your report. We will investigate within 5 working days./, 1, "Text template sent to subscriber";
is $mech->get_text_body_from_email($email_for_reporter) =~ /This is the email alternative \[https:\/\/google.com\]/, 1, "Email template sent to reporter";
is $mech->get_html_body_from_email($email_for_reporter) =~ /<p style="margin: 0 0 16px 0;">\r\nThis is the email <a href\="https:\/\/google\.com">alternative<\/a>.<\/p>/, 1, "Email template text in paragraphs";

my $counciluser = $mech->create_user_ok('[email protected]', name => 'Council User', from_body => $body, is_superuser => 1);
$mech->log_in_ok($counciluser->email);
$mech->get_ok("/admin/report_edit/$report_id");
$mech->content_contains('This is the email ', "Extra template email text displayed");
my $update_id = $report->comments->first->id;
$mech->get_ok("/admin/update_edit/$update_id");
$mech->content_contains("Template email response:", "Template email input present");
$mech->content_contains("This is the email ", "Template email input populated");
$report->comments->first->delete;
$update_id = $mech->create_comment_for_problem($report, $comment_user, 'User', 'Non-template update', 0, 'confirmed', 'confirmed')->id;
$mech->get_ok("/admin/report_edit/$report_id");
$mech->content_contains("Non-template update", 'Standard update text visible');
$mech->content_lacks("Template email response:", 'Template email munged text not added');
$mech->get_ok("/admin/update_edit/$update_id");
$mech->content_contains("Text:", 'Text box shown for standard update');
$mech->content_lacks("Template email response:", 'Email text box not shown for standard update');
};

done_testing;
Expand Down
Loading

0 comments on commit 297465d

Please sign in to comment.