Consolidate sending backoff for different types of sender.

5 files changed, 11 insertions, 62 deletions

diff --git a/bin/send-comments b/bin/send-comments
index fc2ab42bd..3549113c9 100755
--- a/bin/send-comments
+++ b/bin/send-comments
@@ -26,9 +26,6 @@ use mySociety::EmailUtil;
 
 use Open311;
 
-# maximum number of webservice attempts to send before not trying any more (XXX may be better in config?)
-use constant SEND_FAIL_RETRIES_CUTOFF => 3;
-
 # send_method config values found in by-area config data, for selecting to appropriate method
 use constant SEND_METHOD_EMAIL      => 'email';
 use constant SEND_METHOD_OPEN311    => 'Open311';
diff --git a/perllib/FixMyStreet/DB/ResultSet/Problem.pm b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
index dc1c5c248..07848d782 100644
--- a/perllib/FixMyStreet/DB/ResultSet/Problem.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
@@ -236,7 +236,6 @@ sub send_reports {
 
     my $send_report = FixMyStreet::SendReport->new();
     my $senders = $send_report->get_senders;
-    my %sending_skipped_by_method;
 
     my $debug_unsent_count = 0;
     debug_print("starting to loop through unsent problem reports...") if $debug_mode;
@@ -348,8 +347,6 @@ sub send_reports {
 
                 if ( $reporters{ $sender }->should_skip( $row ) ) {
                     debug_print("skipped by sender " . $sender_info->{method} . " (might be due to previous failed attempts?)", $row->id) if $debug_mode;
-                    $sending_skipped_by_method{ $sender }++ if
-                        $reporters{ $sender }->skipped;
                 } else {
                     debug_print("OK, adding recipient body " . $body->id . ":" . $body->name . ", " . $body->send_method, $row->id) if $debug_mode;
                     push @dear, $body->name;
@@ -459,15 +456,6 @@ sub send_reports {
                 print "    " . $notgot{$e}{$c} . " problem, to $e category $c (" . $note{$e}{$c}. ")\n";
             }
         }
-        if (keys %sending_skipped_by_method) {
-            my $c = 0;
-            print "\nProblem reports that send-reports did not attempt to send the following:\n";
-            foreach my $send_method (sort keys %sending_skipped_by_method) {
-                printf "    %-24s %4d\n", "$send_method:", $sending_skipped_by_method{$send_method};
-                $c+=$sending_skipped_by_method{$send_method};
-            }
-            printf "    %-24s %4d\n", "Total:", $c;
-        }
         my $sending_errors = '';
         my $unsent = FixMyStreet::App->model("DB::Problem")->search( {
             state => [ 'confirmed', 'fixed' ],
diff --git a/perllib/FixMyStreet/SendReport.pm b/perllib/FixMyStreet/SendReport.pm
index f8901c6f8..5087c7ead 100644
--- a/perllib/FixMyStreet/SendReport.pm
+++ b/perllib/FixMyStreet/SendReport.pm
@@ -12,13 +12,22 @@ has 'bodies' => ( is => 'rw', isa => 'ArrayRef', default => sub { [] } );
 has 'to' => ( is => 'rw', isa => 'ArrayRef', default => sub { [] } );
 has 'success' => ( is => 'rw', isa => 'Bool', default => 0 );
 has 'error' => ( is => 'rw', isa => 'Str', default => '' );
-has 'skipped' => ( 'is' => 'rw', isa => 'Str', default => '' );
 has 'unconfirmed_counts' => ( 'is' => 'rw', isa => 'HashRef', default => sub { {} } );
 has 'unconfirmed_notes' => ( 'is' => 'rw', isa => 'HashRef', default => sub { {} } );
 
 
 sub should_skip {
-    return 0;
+    my $self = shift;
+    my $row  = shift;
+
+    return 0 unless $row->send_fail_count;
+
+    my $tz = DateTime::TimeZone->new( name => 'local' );
+    my $now = DateTime->now( time_zone => $tz );
+    my $diff = $now - $row->send_fail_timestamp;
+
+    my $backoff = $row->send_fail_count > 1 ? 30 : 5;
+    return $diff->in_units( 'minutes' ) < $backoff;
 }
 
 sub get_senders {
diff --git a/perllib/FixMyStreet/SendReport/Barnet.pm b/perllib/FixMyStreet/SendReport/Barnet.pm
index 05ca20809..68bc8d6dc 100644
--- a/perllib/FixMyStreet/SendReport/Barnet.pm
+++ b/perllib/FixMyStreet/SendReport/Barnet.pm
@@ -10,31 +10,10 @@ use Utils;
 use mySociety::Config;
 use mySociety::Web qw(ent);
 
-# maximum number of webservice attempts to send before not trying any more (XXX may be better in config?)
-use constant SEND_FAIL_RETRIES_CUTOFF => 3;
-
 # specific council numbers
 use constant COUNCIL_ID_BARNET     => 2489;
 use constant MAX_LINE_LENGTH       => 132;
 
-sub should_skip {
-    my $self = shift;
-    my $row = shift;
-
-    my $council_name = 'Barnet';
-    my $err_msg = "";
-
-    if ($row->send_fail_count >= SEND_FAIL_RETRIES_CUTOFF) {
-        $council_name &&= " to $council_name";
-        $err_msg = "skipped: problem id=" . $row->id . " send$council_name has failed " 
-                . $row->send_fail_count . " times, cutoff is " . SEND_FAIL_RETRIES_CUTOFF;
-
-        $self->skipped( $err_msg );
-
-        return 1;
-    }
-}
-
 sub construct_message {
     my %h = @_;
     my $message = <<EOF;
diff --git a/perllib/FixMyStreet/SendReport/Open311.pm b/perllib/FixMyStreet/SendReport/Open311.pm
index 1ad7f5b05..9cdb7c4b4 100644
--- a/perllib/FixMyStreet/SendReport/Open311.pm
+++ b/perllib/FixMyStreet/SendReport/Open311.pm
@@ -13,17 +13,6 @@ use Readonly;
 
 Readonly::Scalar my $COUNCIL_ID_OXFORDSHIRE => 2237;
 
-sub should_skip {
-    my $self = shift;
-    my $row  = shift;
-
-    if ( $row->send_fail_count > 0 ) {
-        if ( bromley_retry_timeout($row) ) {
-            return 1;
-        }
-    }
-}
-
 sub send {
     my $self = shift;
     my ( $row, $h ) = @_;
@@ -164,17 +153,4 @@ sub send {
     return $result;
 }
 
-sub bromley_retry_timeout {
-    my $row = shift;
-
-    my $tz = DateTime::TimeZone->new( name => 'local' );
-    my $now = DateTime->now( time_zone => $tz );
-    my $diff = $now - $row->send_fail_timestamp;
-    if ( $diff->in_units( 'minutes' ) < 30 ) {
-        return 1;
-    }
-
-    return 0;
-}
-
 1;