aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CHANGELOG.md1
-rw-r--r--perllib/FixMyStreet/DB/Result/Comment.pm11
-rw-r--r--perllib/FixMyStreet/DB/ResultSet/State.pm6
-rw-r--r--perllib/FixMyStreet/Script/Alerts.pm15
-rw-r--r--t/app/controller/alert_new.t181
5 files changed, 205 insertions, 9 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3a39807e3..82d0e3ee1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@
- Fix encoded entities in RSS output. #1859
- Only save category changes if staff user update valid #1857
- Only create one update when staff user updating category #1857
+ - Do not include blank updates in email alerts #1857
- Admin improvements:
- Character length limit can be placed on report detailed information #1848
- Inspector panel shows nearest address if available #1850
diff --git a/perllib/FixMyStreet/DB/Result/Comment.pm b/perllib/FixMyStreet/DB/Result/Comment.pm
index 562f29693..409a6e1ab 100644
--- a/perllib/FixMyStreet/DB/Result/Comment.pm
+++ b/perllib/FixMyStreet/DB/Result/Comment.pm
@@ -274,14 +274,9 @@ sub problem_state_display {
return FixMyStreet::DB->resultset("State")->display('confirmed', 1);
} elsif ($self->problem_state) {
my $state = $self->problem_state;
- if ($state eq 'not responsible') {
- $update_state = _( "not the council's responsibility" );
- if ($cobrand eq 'bromley' || $self->problem->to_body_named('Bromley')) {
- $update_state = 'third party responsibility';
- }
- } else {
- $update_state = FixMyStreet::DB->resultset("State")->display($state, 1);
- }
+ my $cobrand_name = $cobrand;
+ $cobrand_name = 'bromley' if $self->problem->to_body_named('Bromley');
+ $update_state = FixMyStreet::DB->resultset("State")->display($state, 1, $cobrand_name);
}
return $update_state;
diff --git a/perllib/FixMyStreet/DB/ResultSet/State.pm b/perllib/FixMyStreet/DB/ResultSet/State.pm
index de56c738c..3e6169aeb 100644
--- a/perllib/FixMyStreet/DB/ResultSet/State.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/State.pm
@@ -58,7 +58,7 @@ sub fixed { [ $_[0]->_filter(sub { $_->type eq 'fixed' }) ] }
# This function can be used to return that label's display name.
sub display {
- my ($rs, $label, $single_fixed) = @_;
+ my ($rs, $label, $single_fixed, $cobrand) = @_;
my $unchanging = {
unconfirmed => _("Unconfirmed"),
hidden => _("Hidden"),
@@ -72,6 +72,10 @@ sub display {
};
$label = 'fixed' if $single_fixed && $label =~ /^fixed - (council|user)$/;
return $unchanging->{$label} if $unchanging->{$label};
+ if ($cobrand && $label eq 'not responsible') {
+ return 'third party responsibility' if $cobrand eq 'bromley';
+ return _("not the council's responsibility");
+ }
my ($state) = $rs->_filter(sub { $_->label eq $label });
return $label unless $state;
$state->name($translate_now->{$label}) if $translate_now->{$label};
diff --git a/perllib/FixMyStreet/Script/Alerts.pm b/perllib/FixMyStreet/Script/Alerts.pm
index aefdf82d5..3f26b749b 100644
--- a/perllib/FixMyStreet/Script/Alerts.pm
+++ b/perllib/FixMyStreet/Script/Alerts.pm
@@ -39,6 +39,7 @@ sub send() {
$item_table.name as item_name, $item_table.anonymous as item_anonymous,
$item_table.confirmed as item_confirmed,
$item_table.photo as item_photo,
+ $item_table.problem_state as item_problem_state,
$head_table.*
from alert, $item_table, $head_table
where alert.parameter::integer = $head_table.id
@@ -65,6 +66,7 @@ sub send() {
$query = FixMyStreet::DB->schema->storage->dbh->prepare($query);
$query->execute();
my $last_alert_id;
+ my $last_problem_state = '';
my %data = ( template => $alert_type->template, data => [], schema => $schema );
while (my $row = $query->fetchrow_hashref) {
@@ -87,6 +89,7 @@ sub send() {
parameter => $row->{item_id},
} );
if ($last_alert_id && $last_alert_id != $row->{alert_id}) {
+ $last_problem_state = '';
_send_aggregated_alert_email(%data);
%data = ( template => $alert_type->template, data => [], schema => $schema );
}
@@ -103,6 +106,18 @@ sub send() {
my $url = $cobrand->base_url_for_report($row);
# this is currently only for new_updates
if (defined($row->{item_text})) {
+ # this might throw up the odd false positive but only in cases where the
+ # state has changed and there was already update text
+ if ($row->{item_problem_state} &&
+ !( $last_problem_state eq '' && $row->{item_problem_state} eq 'confirmed' ) &&
+ $last_problem_state ne $row->{item_problem_state}
+ ) {
+ my $state = FixMyStreet::DB->resultset("State")->display($row->{item_problem_state}, 1, $cobrand);
+
+ my $update = _('State changed to:') . ' ' . $state;
+ $row->{item_text} = $row->{item_text} ? $row->{item_text} . "\n\n" . $update :
+ $update;
+ }
next unless $row->{item_text};
if ( $cobrand->moniker ne 'zurich' && $row->{alert_user_id} == $row->{user_id} ) {
# This is an alert to the same user who made the report - make this a login link
diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t
index 33b3ae92b..9f2e1f53a 100644
--- a/t/app/controller/alert_new.t
+++ b/t/app/controller/alert_new.t
@@ -547,6 +547,187 @@ subtest "Test alerts are not sent for no-text updates" => sub {
$mech->delete_user($user3);
};
+subtest "Test no marked as confirmed added to alerts" => sub {
+ $mech->delete_user( 'reporter@example.com' );
+ $mech->delete_user( 'alerts@example.com' );
+
+ my $user1 = $mech->create_user_ok('reporter@example.com', name => 'Reporter User' );
+ my $user2 = $mech->create_user_ok('alerts@example.com', name => 'Alert User' );
+ my $user3 = $mech->create_user_ok('staff@example.com', name => 'Staff User', from_body => $gloucester );
+ my $dt = DateTime->now(time_zone => 'Europe/London')->add(days => 2);
+
+ my $dt_parser = FixMyStreet::App->model('DB')->schema->storage->datetime_parser;
+
+ my $report_time = '2011-03-01 12:00:00';
+ my $report = FixMyStreet::App->model('DB::Problem')->find_or_create( {
+ postcode => 'EH1 1BB',
+ bodies_str => '1',
+ areas => ',11808,135007,14419,134935,2651,20728,',
+ category => 'Street lighting',
+ title => 'Testing',
+ detail => 'Testing Detail',
+ used_map => 1,
+ name => $user1->name,
+ anonymous => 0,
+ state => 'confirmed',
+ confirmed => $dt_parser->format_datetime($dt),
+ lastupdate => $dt_parser->format_datetime($dt),
+ whensent => $dt_parser->format_datetime($dt->clone->add( minutes => 5 )),
+ lang => 'en-gb',
+ service => '',
+ cobrand => 'default',
+ cobrand_data => '',
+ send_questionnaire => 1,
+ latitude => '55.951963',
+ longitude => '-3.189944',
+ user_id => $user1->id,
+ } );
+ my $report_id = $report->id;
+ ok $report, "created test report - $report_id";
+
+ my $alert = FixMyStreet::App->model('DB::Alert')->create( {
+ parameter => $report_id,
+ alert_type => 'new_updates',
+ user => $user2,
+ } )->confirm;
+ ok $alert, 'created alert for other user';
+
+ my $update = FixMyStreet::App->model('DB::Comment')->create( {
+ problem_id => $report_id,
+ user_id => $user3->id,
+ name => 'Staff User',
+ mark_fixed => 'false',
+ text => 'this is update',
+ state => 'confirmed',
+ problem_state => 'confirmed',
+ confirmed => $dt->clone->add( hours => 9 ),
+ anonymous => 'f',
+ } );
+ my $update_id = $update->id;
+ ok $update, "created test update from staff user - $update_id";
+
+ $mech->clear_emails_ok;
+ FixMyStreet::override_config {
+ MAPIT_URL => 'http://mapit.uk/',
+ }, sub {
+ FixMyStreet::Script::Alerts::send();
+ };
+
+ $mech->email_count_is(1);
+ my $email = $mech->get_email;
+ my $body = $mech->get_text_body_from_email($email);
+ like $body, qr/The following updates have been left on this report:/, 'email is about updates to existing report';
+ like $body, qr/Staff User/, 'Update comes from correct user';
+ unlike $body, qr/State changed to: Open/s, 'no marked as confirmed text';
+
+ $mech->delete_user($user1);
+ $mech->delete_user($user2);
+ $mech->delete_user($user3);
+};
+
+for my $test (
+ {
+ update_text => '',
+ problem_state => 'investigating',
+ expected_text => 'State changed to: Investigating',
+ desc => 'comment changing status included in email',
+ },
+ {
+ update_text => 'Category changed to Potholes',
+ problem_state => '',
+ expected_text => 'Category changed to Potholes',
+ desc => 'comment about category included',
+ },
+ {
+ update_text => 'Category changed to Potholes',
+ problem_state => 'investigating',
+ expected_text => 'Category changed to Potholes.*Investigating',
+ desc => 'comment about category and status change included',
+ },
+) {
+ subtest $test->{desc} => sub {
+ $mech->delete_user( 'reporter@example.com' );
+ $mech->delete_user( 'alerts@example.com' );
+
+ my $user1 = $mech->create_user_ok('reporter@example.com', name => 'Reporter User' );
+ my $user2 = $mech->create_user_ok('alerts@example.com', name => 'Alert User' );
+ my $user3 = $mech->create_user_ok('staff@example.com', name => 'Staff User', from_body => $gloucester );
+ my $dt = DateTime->now(time_zone => 'Europe/London')->add(days => 2);
+
+ my $dt_parser = FixMyStreet::App->model('DB')->schema->storage->datetime_parser;
+
+ my $report_time = '2011-03-01 12:00:00';
+ my $report = FixMyStreet::App->model('DB::Problem')->find_or_create( {
+ postcode => 'EH1 1BB',
+ bodies_str => '1',
+ areas => ',11808,135007,14419,134935,2651,20728,',
+ category => 'Street lighting',
+ title => 'Testing',
+ detail => 'Testing Detail',
+ used_map => 1,
+ name => $user1->name,
+ anonymous => 0,
+ state => 'confirmed',
+ confirmed => $dt_parser->format_datetime($dt),
+ lastupdate => $dt_parser->format_datetime($dt),
+ whensent => $dt_parser->format_datetime($dt->clone->add( minutes => 5 )),
+ lang => 'en-gb',
+ service => '',
+ cobrand => 'default',
+ cobrand_data => '',
+ send_questionnaire => 1,
+ latitude => '55.951963',
+ longitude => '-3.189944',
+ user_id => $user1->id,
+ } );
+ my $report_id = $report->id;
+ ok $report, "created test report - $report_id";
+
+ my $alert = FixMyStreet::App->model('DB::Alert')->create( {
+ parameter => $report_id,
+ alert_type => 'new_updates',
+ user => $user2,
+ } )->confirm;
+ ok $alert, 'created alert for other user';
+
+ my $update = FixMyStreet::App->model('DB::Comment')->create( {
+ problem_id => $report_id,
+ user_id => $user3->id,
+ name => 'Staff User',
+ mark_fixed => 'false',
+ text => $test->{update_text},
+ problem_state => $test->{problem_state},
+ state => 'confirmed',
+ confirmed => $dt->clone->add( hours => 9 ),
+ anonymous => 'f',
+ } );
+ my $update_id = $update->id;
+ ok $update, "created test update from staff user - $update_id";
+
+ $mech->clear_emails_ok;
+ FixMyStreet::override_config {
+ MAPIT_URL => 'http://mapit.uk/',
+ }, sub {
+ FixMyStreet::Script::Alerts::send();
+ };
+
+ $mech->email_count_is(1);
+ my $expected_text = $test->{expected_text};
+ my $email = $mech->get_email;
+ my $body = $mech->get_text_body_from_email($email);
+ like $body, qr/The following updates have been left on this report:/, 'email is about updates to existing report';
+ like $body, qr/Staff User/, 'Update comes from correct user';
+ like $body, qr/$expected_text/s, 'Expected text present';
+
+ my @urls = $mech->get_link_from_email($email, 1);
+ is $urls[0], "http://www.example.org/report/" . $report_id, "Correct report URL in email";
+
+ $mech->delete_user($user1);
+ $mech->delete_user($user2);
+ $mech->delete_user($user3);
+ };
+}
+
subtest "Test signature template is used from cobrand" => sub {
$mech->delete_user( 'reporter@example.com' );
$mech->delete_user( 'alerts@example.com' );
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-18 18:02:42 +0000