Skip to content

Commit

Permalink
Fix more over-display of "State changed" in alerts
Browse files Browse the repository at this point in the history 
My previous fix did not work when the script moves from one alert ID to
another.
  • Loading branch information
@dracos
dracos committed Oct 20, 2021
1 parent f4cc170 commit a6e7516
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 15 deletions.
5 changes: 4 additions & 1 deletion perllib/FixMyStreet/Script/Alerts.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ sub send_alert_type {
parameter => $row->{item_id},
} );

if ($last_alert_id && $last_alert_id != $row->{alert_id}) {
$last_problem_state = 'confirmed';
}

# this is currently only for new_updates
if ($row->{is_new_update}) {
# this might throw up the odd false positive but only in cases where the
Expand All @@ -125,7 +129,6 @@ sub send_alert_type {
}

if ($last_alert_id && $last_alert_id != $row->{alert_id}) {
$last_problem_state = '';
_send_aggregated_alert(%data);
%data = ( template => $alert_type->template, data => [], schema => $schema );
}
Expand Down
37 changes: 23 additions & 14 deletions t/app/controller/alert_new.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ subtest "Test normal alert signups and that alerts are sent" => sub {

my $dt = DateTime->now()->add(days => 2);

my ($report) = $mech->create_problems_for_body(1, 1, 'Testing', {
my ($report) = $mech->create_problems_for_body(1, $body->id, 'Testing', {
dt => $dt,
user => $user1,
postcode => 'EH1 1BB',
Expand Down Expand Up @@ -531,7 +531,7 @@ subtest "Test alerts are not sent for no-text updates" => sub {
my $user3 = $mech->create_user_ok('staff@example.com', name => 'Staff User', from_body => $gloucester );
my $dt = DateTime->now()->add(days => 2);

my ($report, $report2) = $mech->create_problems_for_body(2, 1, 'Testing', {
my ($report, $report2) = $mech->create_problems_for_body(2, $body->id, 'Testing', {
user => $user1,
});
my $report_id = $report->id;
Expand Down Expand Up @@ -579,23 +579,28 @@ subtest "Test no marked as confirmed added to alerts" => sub {
my $user3 = $mech->create_user_ok('staff@example.com', name => 'Staff User', from_body => $gloucester );
my $dt = DateTime->now()->add(days => 2);

my ($report) = $mech->create_problems_for_body(1, 1, 'Testing', {
my ($report1, $report2) = $mech->create_problems_for_body(2, $body->id, 'Testing', {
user => $user1,
state => 'investigating',
});
my $report_id = $report->id;
ok $report, "created test report - $report_id";
my $report_id = $report1->id;
ok $report1, "created test reports - $report_id";

my $alert = FixMyStreet::DB->resultset('Alert')->create( {
my $alert1 = FixMyStreet::DB->resultset('Alert')->create( {
parameter => $report_id,
alert_type => 'new_updates',
user => $user2,
} )->confirm;
ok $alert, 'created alert for other user';
my $alert2 = FixMyStreet::DB->resultset('Alert')->create( {
parameter => $report2->id,
alert_type => 'new_updates',
user => $user2,
} )->confirm;

$mech->create_comment_for_problem($report, $user3, 'Staff User', 'this is update', 'f', 'confirmed', 'confirmed', { confirmed => $dt->clone->add( hours => 9 ) });
$mech->create_comment_for_problem($report, $user3, 'Staff User', 'this is another update', 'f', 'confirmed', 'investigating', { confirmed => $dt->clone->add( hours => 10 ) });
$mech->create_comment_for_problem($report, $user3, 'Staff User', 'this is a third update, same state', 'f', 'confirmed', 'investigating', { confirmed => $dt->clone->add( hours => 11 ) });
$mech->create_comment_for_problem($report1, $user3, 'Staff User', 'this is update', 'f', 'confirmed', 'confirmed', { confirmed => $dt->clone->add( hours => 9 ) });
$mech->create_comment_for_problem($report1, $user3, 'Staff User', 'this is another update', 'f', 'confirmed', 'investigating', { confirmed => $dt->clone->add( hours => 10 ) });
$mech->create_comment_for_problem($report1, $user3, 'Staff User', 'this is a third update, same state', 'f', 'confirmed', 'investigating', { confirmed => $dt->clone->add( hours => 11 ) });
$mech->create_comment_for_problem($report2, $user3, 'Staff User', 'this is update', 'f', 'confirmed', 'confirmed', { confirmed => $dt->clone->add( hours => 9 ) });

$mech->clear_emails_ok;
FixMyStreet::override_config {
Expand All @@ -604,14 +609,18 @@ subtest "Test no marked as confirmed added to alerts" => sub {
FixMyStreet::Script::Alerts::send_updates();
};

$mech->email_count_is(1);
my $email = $mech->get_email;
my $body = $mech->get_text_body_from_email($email);
$mech->email_count_is(2);
my @email = $mech->get_email;
my $body = $mech->get_text_body_from_email($email[0]);
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';
like $body, qr/State changed to: Investigating/, 'mention of state change';
unlike $body, qr/State changed to: Investigating.*State changed to: Investigating/s, 'only one mention of state change';
$body = $mech->get_text_body_from_email($email[1]);
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);
Expand Down Expand Up @@ -644,7 +653,7 @@ for my $test (
my $user3 = $mech->create_user_ok('staff@example.com', name => 'Staff User', from_body => $gloucester );
my $dt = DateTime->now()->add(days => 2);

my ($report) = $mech->create_problems_for_body(1, 1, 'Testing', {
my ($report) = $mech->create_problems_for_body(1, $body->id, 'Testing', {
user => $user1,
});
my $report_id = $report->id;
Expand Down

0 comments on commit a6e7516

Please sign in to comment.