simplfy state change code and also correct bugs

2 files changed, 83 insertions, 23 deletions

diff --git a/perllib/Open311/GetServiceRequestUpdates.pm b/perllib/Open311/GetServiceRequestUpdates.pm
index c9bb7a91b..c0b9dcac0 100644
--- a/perllib/Open311/GetServiceRequestUpdates.pm
+++ b/perllib/Open311/GetServiceRequestUpdates.pm
@@ -96,23 +96,16 @@ sub update_comments {
                 # do not change the status of the problem as it's
                 # tricky to determine the right thing to do.
                 if ( $comment->created_local > $p->lastupdate_local ) {
-                    my $incoming_status = lc( $request->{status} );
-                    my $internal_status = $incoming_status;
-                    $internal_status =~ s/_/ /g;
-
-                    $internal_status = 'not responsible' if $internal_status eq 'not councils responsibility';
-                    if ( $p->is_open and ( $incoming_status eq 'closed' or $incoming_status eq 'fixed' ) ) {
-                        $p->state( 'fixed - council' );
-                        $comment->problem_state( 'fixed - council' );
-                    } elsif ( ( $p->is_closed || $p->is_fixed ) and (
-                            $incoming_status eq 'open' or FixMyStreet::DB::Result::Problem->open_states()->{ $internal_status }
-                        ) ) {
-                        $p->state( 'confirmed' );
-                        $comment->problem_state( 'confirmed' );
-                    } else {
-                        if ( FixMyStreet::DB::Result::Problem->council_states()->{ $internal_status } ) {
-                            $p->state( $internal_status );
-                            $comment->problem_state( $internal_status );
+                    my $state = $self->map_state( $request->{status} );
+
+                    # do not change a fixed problem to a closed one
+                    if ( $p->is_fixed && FixMyStreet::DB::Result::Problem->open_states()->{$state} ) {
+                        $p->state($state);
+                        $comment->problem_state($state);
+                    } elsif ( ( $p->is_open || $p->is_closed ) && $p->state ne $state ) {
+                        if ( FixMyStreet::DB::Result::Problem->council_states() ->{$state} ) {
+                            $p->state($state);
+                            $comment->problem_state($state);
                         }
                     }
                 }
@@ -141,4 +134,21 @@ sub update_comments {
     return 1;
 }
 
+sub map_state {
+    my $self           = shift;
+    my $incoming_state = shift;
+
+    $incoming_state = lc($incoming_state);
+    $incoming_state =~ s/_/ /g;
+
+    my %state_map = (
+        fixed                         => 'fixed - council',
+        'not councils responsibility' => 'not responsible',
+        open                          => 'confirmed',
+        closed                        => 'fixed - council'
+    );
+
+    return $state_map{$incoming_state} || $incoming_state;
+}
+
 1;
diff --git a/t/open311/getservicerequestupdates.t b/t/open311/getservicerequestupdates.t
index 7930c66a2..680ea930c 100644
--- a/t/open311/getservicerequestupdates.t
+++ b/t/open311/getservicerequestupdates.t
@@ -105,7 +105,7 @@ $problem->insert;
 
 for my $test (
     {
-        desc => 'element with content',
+        desc => 'OPEN status for confirmed problem does not change state',
         updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
         description => 'This is a note',
         external_id => 638344,
@@ -117,6 +117,19 @@ for my $test (
         end_state => 'confirmed',
     },
     {
+        desc => 'bad state does not update states but does create update',
+        updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
+        description => 'This is a note',
+        external_id => 638344,
+        start_state => 'confirmed',
+        comment_status => 'INVALID_STATE',
+        mark_fixed=> 0,
+        mark_open => 0,
+        problem_state => undef,
+        end_state => 'confirmed',
+    },
+
+    {
         desc => 'investigating status changes problem status',
         updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
         description => 'This is a note',
@@ -177,7 +190,7 @@ for my $test (
         end_state => 'duplicate',
     },
     {
-        desc => 'fixed status changes problem status',
+        desc => 'fixed status marks report as fixed - council',
         updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
         description => 'This is a note',
         external_id => 638344,
@@ -189,7 +202,7 @@ for my $test (
         end_state => 'fixed - council',
     },
     {
-        desc => 'comment closes report',
+        desc => 'status of CLOSED marks report as fixed - council',
         updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
         description => 'This is a note',
         external_id => 638344,
@@ -201,7 +214,7 @@ for my $test (
         end_state => 'fixed - council',
     },
     {
-        desc => 'comment re-opens fixed report',
+        desc => 'status of OPEN re-opens fixed report',
         updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
         description => 'This is a note',
         external_id => 638344,
@@ -213,7 +226,19 @@ for my $test (
         end_state => 'confirmed',
     },
     {
-        desc => 'comment re-opens closed report',
+        desc => 'action sheduled re-opens fixed report as action scheduled',
+        updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
+        description => 'This is a note',
+        external_id => 638344,
+        start_state => 'fixed - user',
+        comment_status => 'ACTION_SCHEDULED',
+        mark_fixed => 0,
+        mark_open => 0,
+        problem_state => 'action scheduled',
+        end_state => 'action scheduled',
+    },
+    {
+        desc => 'open status re-opens closed report',
         updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
         description => 'This is a note',
         external_id => 638344,
@@ -225,7 +250,7 @@ for my $test (
         end_state => 'confirmed',
     },
     {
-        desc => 'comment leaves report closed',
+        desc => 'fixed status leaves fixed - user report as fixed - user',
         updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
         description => 'This is a note',
         external_id => 638344,
@@ -233,8 +258,33 @@ for my $test (
         comment_status => 'FIXED',
         mark_fixed => 0,
         mark_open => 0,
+        problem_state => undef,
+        end_state => 'fixed - user',
+    },
+    {
+        desc => 'closed status leaves fixed report as fixed',
+        updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
+        description => 'This is a note',
+        external_id => 638344,
+        start_state => 'fixed - user',
+        comment_status => 'UNABLE_TO_FIX',
+        mark_fixed => 0,
+        mark_open => 0,
+        problem_state => undef,
         end_state => 'fixed - user',
     },
+    {
+        desc => 'fixed status sets closed report as fixed',
+        updated_datetime => sprintf( '<updated_datetime>%s</updated_datetime>', $dt ),
+        description => 'This is a note',
+        external_id => 638344,
+        start_state => 'unable to fix',
+        comment_status => 'FIXED',
+        mark_fixed => 0,
+        mark_open => 0,
+        problem_state => 'fixed - council',
+        end_state => 'fixed - council',
+    },
 ) {
     subtest $test->{desc} => sub {
         my $local_requests_xml = $requests_xml;