diff options
Diffstat (limited to 'perllib/Open311')
| -rw-r--r-- | perllib/Open311/GetServiceRequestUpdates.pm | 373 | ||||
| -rw-r--r-- | perllib/Open311/GetServiceRequests.pm | 28 | ||||
| -rw-r--r-- | perllib/Open311/PopulateServiceList.pm | 120 | ||||
| -rwxr-xr-x | perllib/Open311/PostServiceRequestUpdates.pm | 117 |
4 files changed, 401 insertions, 237 deletions
diff --git a/perllib/Open311/GetServiceRequestUpdates.pm b/perllib/Open311/GetServiceRequestUpdates.pm index 3b436aaa7..9fa81ac9e 100644 --- a/perllib/Open311/GetServiceRequestUpdates.pm +++ b/perllib/Open311/GetServiceRequestUpdates.pm @@ -2,6 +2,7 @@ package Open311::GetServiceRequestUpdates; use Moo; use Open311; +use Parallel::ForkManager; use FixMyStreet::DB; use FixMyStreet::App::Model::PhotoSet; use DateTime::Format::W3CDTF; @@ -15,6 +16,9 @@ has verbose => ( is => 'ro', default => 0 ); has schema => ( is =>'ro', lazy => 1, default => sub { FixMyStreet::DB->schema->connect } ); has blank_updates_permitted => ( is => 'rw', default => 0 ); +has current_body => ( is => 'rw' ); +has current_open311 => ( is => 'rw' ); + Readonly::Scalar my $AREA_ID_BROMLEY => 2482; Readonly::Scalar my $AREA_ID_OXFORDSHIRE => 2237; @@ -34,7 +38,30 @@ sub fetch { $bodies = $bodies->search( { name => $self->body } ); } + my $procs_min = FixMyStreet->config('FETCH_COMMENTS_PROCESSES_MIN') || 0; + my $procs_max = FixMyStreet->config('FETCH_COMMENTS_PROCESSES_MAX'); + my $procs_timeout = FixMyStreet->config('FETCH_COMMENTS_PROCESS_TIMEOUT'); + + my $pm = Parallel::ForkManager->new(FixMyStreet->test_mode ? 0 : $procs_min); + + if ($procs_max && $procs_timeout) { + my %workers; + $pm->run_on_wait(sub { + while (my ($pid, $started_at) = each %workers) { + next unless time() - $started_at > $procs_timeout; + next if $pm->max_procs == $procs_max; + $pm->set_max_procs($pm->max_procs + 1); + delete $workers{$pid}; # Only want to increase once per long-running thing + } + }, 1); + $pm->run_on_start(sub { my $pid = shift; $workers{$pid} = time(); }); + $pm->run_on_finish(sub { my $pid = shift; delete $workers{$pid}; }); + } + while ( my $body = $bodies->next ) { + $pm->start and next; + + $self->current_body( $body ); my %open311_conf = ( endpoint => $body->endpoint, @@ -47,37 +74,52 @@ sub fetch { $cobrand->call_hook(open311_config_updates => \%open311_conf) if $cobrand; - my $o = $open311 || Open311->new(%open311_conf); + $self->current_open311( $open311 || Open311->new(%open311_conf) ); $self->suppress_alerts( $body->suppress_alerts ); $self->blank_updates_permitted( $body->blank_updates_permitted ); $self->system_user( $body->comment_user ); - $self->update_comments( $o, $body ); + $self->process_body(); + + $pm->finish; } + + $pm->wait_all_children; } -sub update_comments { - my ( $self, $open311, $body ) = @_; +sub parse_dates { + my $self = shift; + my $body = $self->current_body; my @args = (); - if ( $self->start_date || $self->end_date ) { - return 0 unless $self->start_date && $self->end_date; + my $dt = DateTime->now(); + # Oxfordshire uses local time and not UTC for dates + FixMyStreet->set_time_zone($dt) if $body->areas->{$AREA_ID_OXFORDSHIRE}; - push @args, $self->start_date; - push @args, $self->end_date; # default to asking for last 2 hours worth if not Bromley + if ($self->start_date) { + push @args, DateTime::Format::W3CDTF->format_datetime( $self->start_date ); } elsif ( ! $body->areas->{$AREA_ID_BROMLEY} ) { - my $end_dt = DateTime->now(); - # Oxfordshire uses local time and not UTC for dates - FixMyStreet->set_time_zone($end_dt) if ( $body->areas->{$AREA_ID_OXFORDSHIRE} ); - my $start_dt = $end_dt->clone; - $start_dt->add( hours => -2 ); - + my $start_dt = $dt->clone->add( hours => -2 ); push @args, DateTime::Format::W3CDTF->format_datetime( $start_dt ); - push @args, DateTime::Format::W3CDTF->format_datetime( $end_dt ); } + if ($self->end_date) { + push @args, DateTime::Format::W3CDTF->format_datetime( $self->end_date ); + } elsif ( ! $body->areas->{$AREA_ID_BROMLEY} ) { + push @args, DateTime::Format::W3CDTF->format_datetime( $dt ); + } + + return @args; +} + +sub process_body { + my $self = shift; + + my $open311 = $self->current_open311; + my $body = $self->current_body; + my @args = $self->parse_dates; my $requests = $open311->get_service_request_updates( @args ); unless ( $open311->success ) { @@ -87,172 +129,209 @@ sub update_comments { } for my $request (@$requests) { - my $request_id = $request->{service_request_id}; + next unless defined $request->{update_id}; - # If there's no request id then we can't work out - # what problem it belongs to so just skip - next unless $request_id || $request->{fixmystreet_id}; + my $p = $self->find_problem($request, @args) or next; + my $c = $p->comments->search( { external_id => $request->{update_id} } ); + next if $c->first; - my $comment_time = eval { - DateTime::Format::W3CDTF->parse_datetime( $request->{updated_datetime} || "" ) - ->set_time_zone(FixMyStreet->local_time_zone); - }; - next if $@; - my $updated = DateTime::Format::W3CDTF->format_datetime($comment_time->clone->set_time_zone('UTC')); - next if @args && ($updated lt $args[0] || $updated gt $args[1]); - - my $problem; - my $match_field = 'external_id'; - my $criteria = { - external_id => $request_id, + $self->process_update($request, $p); + } + + return 1; +} + +sub check_date { + my ($self, $request, @args) = @_; + + my $comment_time = eval { + DateTime::Format::W3CDTF->parse_datetime( $request->{updated_datetime} || "" ) + ->set_time_zone(FixMyStreet->local_time_zone); + }; + return if $@; + my $updated = DateTime::Format::W3CDTF->format_datetime($comment_time->clone->set_time_zone('UTC')); + return if @args && ($updated lt $args[0] || $updated gt $args[1]); + $request->{comment_time} = $comment_time; + return 1; +} + +sub find_problem { + my ($self, $request, @args) = @_; + + $self->check_date($request, @args) or return; + + my $body = $self->current_body; + my $request_id = $request->{service_request_id}; + + # If there's no request id then we can't work out + # what problem it belongs to so just skip + return unless $request_id || $request->{fixmystreet_id}; + + my $problem; + my $criteria = { + external_id => $request_id, + }; + + # in some cases we only have the FMS id and not the request id so use that + if ( $request->{fixmystreet_id} ) { + unless ( $request->{fixmystreet_id} =~ /^\d+$/ ) { + warn "skipping bad fixmystreet id in updates for " . $body->name . ": [" . $request->{fixmystreet_id} . "], external id is $request_id\n"; + return; + } + + $criteria = { + id => $request->{fixmystreet_id}, }; + } - # in some cases we only have the FMS id and not the request id so use that - if ( $request->{fixmystreet_id} ) { - unless ( $request->{fixmystreet_id} =~ /^\d+$/ ) { - warn "skipping bad fixmystreet id in updates for " . $body->name . ": [" . $request->{fixmystreet_id} . "], external id is $request_id\n"; - next; - } + $problem = $self->schema->resultset('Problem')->to_body($body)->search( $criteria ); + return $problem->first; +} - $criteria = { - id => $request->{fixmystreet_id}, - }; - $match_field = 'fixmystreet id'; +sub process_update { + my ($self, $request, $p) = @_; + my $open311 = $self->current_open311; + my $body = $self->current_body; + + my $state = $open311->map_state( $request->{status} ); + my $old_state = $p->state; + my $external_status_code = $request->{external_status_code} || ''; + my $customer_reference = $request->{customer_reference} || ''; + my $old_external_status_code = $p->get_extra_metadata('external_status_code') || ''; + my $comment = $self->schema->resultset('Comment')->new( + { + problem => $p, + user => $self->system_user, + external_id => $request->{update_id}, + text => $self->comment_text_for_request( + $request, $p, $state, $old_state, + $external_status_code, $old_external_status_code + ), + mark_fixed => 0, + mark_open => 0, + anonymous => 0, + name => $self->system_user->name, + confirmed => $request->{comment_time}, + created => $request->{comment_time}, + state => 'confirmed', } + ); - $problem = $self->schema->resultset('Problem')->to_body($body)->search( $criteria ); - - if (my $p = $problem->first) { - next unless defined $request->{update_id}; - my $c = $p->comments->search( { external_id => $request->{update_id} } ); - - if ( !$c->first ) { - my $state = $open311->map_state( $request->{status} ); - my $old_state = $p->state; - my $external_status_code = $request->{external_status_code} || ''; - my $customer_reference = $request->{customer_reference} || ''; - my $old_external_status_code = $p->get_extra_metadata('external_status_code') || ''; - my $comment = $self->schema->resultset('Comment')->new( - { - problem => $p, - user => $self->system_user, - external_id => $request->{update_id}, - text => $self->comment_text_for_request( - $request, $p, $state, $old_state, - $external_status_code, $old_external_status_code - ), - mark_fixed => 0, - mark_open => 0, - anonymous => 0, - name => $self->system_user->name, - confirmed => $comment_time, - created => $comment_time, - state => 'confirmed', - } - ); - - # Some Open311 services, e.g. Confirm via open311-adapter, provide - # a more fine-grained status code that we use within FMS for - # response templates. - if ( $external_status_code ) { - $comment->set_extra_metadata(external_status_code => $external_status_code); - $p->set_extra_metadata(external_status_code => $external_status_code); - } - - # if the customer reference to display in the report metadata is - # not the same as the external_id - if ( $customer_reference ) { - $p->set_extra_metadata( customer_reference => $customer_reference ); - } - - $open311->add_media($request->{media_url}, $comment) - if $request->{media_url}; - - # don't update state unless it's an allowed state - if ( FixMyStreet::DB::Result::Problem->visible_states()->{$state} && - # For Oxfordshire, don't allow changes back to Open from other open states - !( $body->areas->{$AREA_ID_OXFORDSHIRE} && $state eq 'confirmed' && $p->is_open ) && - # Don't let it change between the (same in the front end) fixed states - !( $p->is_fixed && FixMyStreet::DB::Result::Problem->fixed_states()->{$state} ) ) { - - $comment->problem_state($state); - - # if the comment is older than the last update do not - # change the status of the problem as it's tricky to - # determine the right thing to do. Allow the same time in - # case report/update created at same time (in external - # system). Only do this if the report is currently visible. - if ( $comment->created >= $p->lastupdate && $p->state ne $state && $p->is_visible ) { - $p->state($state); - } - } - - # If nothing to show (no text, photo, or state change), don't show this update - $comment->state('hidden') unless $comment->text || $comment->photo - || ($comment->problem_state && $state ne $old_state); - - # As comment->created has been looked at above, its time zone has been shifted - # to TIME_ZONE (if set). We therefore need to set it back to local before - # insertion. We also then need a clone, otherwise the setting of lastupdate - # will *also* reshift comment->created's time zone to TIME_ZONE. - my $created = $comment->created->set_time_zone(FixMyStreet->local_time_zone); - $p->lastupdate($created->clone); - $p->update; - $comment->insert(); - - if ( $self->suppress_alerts ) { - my @alerts = $self->schema->resultset('Alert')->search( { - alert_type => 'new_updates', - parameter => $p->id, - confirmed => 1, - user_id => $p->user->id, - } ); - - for my $alert (@alerts) { - my $alerts_sent = $self->schema->resultset('AlertSent')->find_or_create( { - alert_id => $alert->id, - parameter => $comment->id, - } ); - } - } - } - # we get lots of comments that are not related to FMS issues from Lewisham so ignore those otherwise - # way too many warnings. - } elsif (FixMyStreet->config('STAGING_SITE') and $body->name !~ /Lewisham/) { - warn "Failed to match comment to problem with $match_field $request_id for " . $body->name . "\n"; + # Some Open311 services, e.g. Confirm via open311-adapter, provide + # a more fine-grained status code that we use within FMS for + # response templates. + if ( $external_status_code ) { + $comment->set_extra_metadata(external_status_code => $external_status_code); + $p->set_extra_metadata(external_status_code => $external_status_code); + } else { + $p->set_extra_metadata(external_status_code => ''); + } + + # if the customer reference to display in the report metadata is + # not the same as the external_id + if ( $customer_reference ) { + $p->set_extra_metadata( customer_reference => $customer_reference ); + } + + $open311->add_media($request->{media_url}, $comment) + if $request->{media_url}; + + # don't update state unless it's an allowed state + if ( FixMyStreet::DB::Result::Problem->visible_states()->{$state} && + # For Oxfordshire, don't allow changes back to Open from other open states + !( $body->areas->{$AREA_ID_OXFORDSHIRE} && $state eq 'confirmed' && $p->is_open ) && + # Don't let it change between the (same in the front end) fixed states + !( $p->is_fixed && FixMyStreet::DB::Result::Problem->fixed_states()->{$state} ) ) { + + $comment->problem_state($state); + + # we only want to update the problem state if that makes sense. We never want to unhide a problem. + # If the update is older than the last update then we also do not want to update the state. This + # is largely to avoid the situation where we miss some updates, make more updates and then catch + # the updates when we fetch the last 24 hours of updates. The exception to this is the first + # comment. This is to catch automated updates which happen faster than we get the external_id + # back from the endpoint and hence have an created time before the lastupdate. + if ( $p->is_visible && $p->state ne $state && + ( $comment->created >= $p->lastupdate || $p->comments->count == 0 ) ) { + $p->state($state); } } - return 1; + # If nothing to show (no text, photo, or state change), don't show this update + $comment->state('hidden') unless $comment->text || $comment->photo + || ($comment->problem_state && $state ne $old_state); + + my $cobrand = $body->get_cobrand_handler; + $cobrand->call_hook(open311_get_update_munging => $comment) + if $cobrand; + + # As comment->created has been looked at above, its time zone has been shifted + # to TIME_ZONE (if set). We therefore need to set it back to local before + # insertion. We also then need a clone, otherwise the setting of lastupdate + # will *also* reshift comment->created's time zone to TIME_ZONE. + my $created = $comment->created->set_time_zone(FixMyStreet->local_time_zone); + $p->lastupdate($created->clone); + $p->update; + $comment->insert(); + + if ( $self->suppress_alerts ) { + my @alerts = $self->schema->resultset('Alert')->search( { + alert_type => 'new_updates', + parameter => $p->id, + confirmed => 1, + user_id => $p->user->id, + } ); + + for my $alert (@alerts) { + my $alerts_sent = $self->schema->resultset('AlertSent')->find_or_create( { + alert_id => $alert->id, + parameter => $comment->id, + } ); + } + } + + return $comment; } sub comment_text_for_request { my ($self, $request, $problem, $state, $old_state, $ext_code, $old_ext_code) = @_; - return $request->{description} if $request->{description}; - # Response templates are only triggered if the state/external status has changed. # And treat any fixed state as fixed. my $state_changed = $state ne $old_state && !( $problem->is_fixed && FixMyStreet::DB::Result::Problem->fixed_states()->{$state} ); my $ext_code_changed = $ext_code ne $old_ext_code; + my $template; if ($state_changed || $ext_code_changed) { + my $order; my $state_params = { 'me.state' => $state }; if ($ext_code) { $state_params->{'me.external_status_code'} = $ext_code; + # make sure that empty string/nulls come last. + $order = { order_by => \"me.external_status_code DESC NULLS LAST" }; }; - if (my $template = $problem->response_templates->search({ + if (my $t = $problem->response_templates->search({ auto_response => 1, -or => $state_params, - })->first) { - return $template->text; + }, $order )->first) { + $template = $t->text; } } + my $desc = $request->{description} || ''; + if ($desc && (!$template || $template !~ /\{\{description}}/)) { + return $desc; + } + + if ($template) { + $template =~ s/\{\{description}}/$desc/; + return $template; + } + return "" if $self->blank_updates_permitted; print STDERR "Couldn't determine update text for $request->{update_id} (report " . $problem->id . ")\n"; diff --git a/perllib/Open311/GetServiceRequests.pm b/perllib/Open311/GetServiceRequests.pm index 194d8d296..e5fd6438e 100644 --- a/perllib/Open311/GetServiceRequests.pm +++ b/perllib/Open311/GetServiceRequests.pm @@ -10,6 +10,7 @@ use DateTime::Format::W3CDTF; has system_user => ( is => 'rw' ); has start_date => ( is => 'ro', default => sub { undef } ); has end_date => ( is => 'ro', default => sub { undef } ); +has body => ( is => 'ro', default => sub { undef } ); has fetch_all => ( is => 'rw', default => 0 ); has verbose => ( is => 'ro', default => 0 ); has schema => ( is =>'ro', lazy => 1, default => sub { FixMyStreet::DB->schema->connect } ); @@ -27,6 +28,10 @@ sub fetch { } ); + if ( $self->body ) { + $bodies = $bodies->search( { name => $self->body } ); + } + while ( my $body = $bodies->next ) { my $o = $self->create_open311_object( $body ); @@ -55,18 +60,17 @@ sub create_problems { my $args = {}; - if ( $self->start_date || $self->end_date ) { - return 0 unless $self->start_date && $self->end_date; - + my $dt = DateTime->now(); + if ($self->start_date) { $args->{start_date} = DateTime::Format::W3CDTF->format_datetime( $self->start_date ); - $args->{end_date} = DateTime::Format::W3CDTF->format_datetime( $self->end_date ); } elsif ( !$self->fetch_all ) { - my $end_dt = DateTime->now(); - my $start_dt = $end_dt->clone; - $start_dt->add( hours => -1 ); + $args->{start_date} = DateTime::Format::W3CDTF->format_datetime( $dt->clone->add(hours => -1) ); + } - $args->{start_date} = DateTime::Format::W3CDTF->format_datetime( $start_dt ); - $args->{end_date} = DateTime::Format::W3CDTF->format_datetime( $end_dt ); + if ($self->end_date) { + $args->{end_date} = DateTime::Format::W3CDTF->format_datetime( $self->end_date ); + } elsif ( !$self->fetch_all ) { + $args->{end_date} = DateTime::Format::W3CDTF->format_datetime( $dt ); } my $requests = $open311->get_service_requests( $args ); @@ -146,7 +150,8 @@ sub create_problems { next; } - if ( my $cobrand = $body->get_cobrand_handler ) { + my $cobrand = $body->get_cobrand_handler; + if ( $cobrand ) { my $filtered = $cobrand->call_hook('filter_report_description', $request->{description}); $request->{description} = $filtered if defined $filtered; } @@ -157,6 +162,7 @@ sub create_problems { my $state = $open311->map_state($request->{status}); my $non_public = $request->{non_public} ? 1 : 0; + $non_public ||= $contacts[0] ? $contacts[0]->non_public : 0; my $problem = $self->schema->resultset('Problem')->new( { @@ -184,6 +190,8 @@ sub create_problems { } ); + next if $cobrand && $cobrand->call_hook(open311_skip_report_fetch => $problem); + $open311->add_media($request->{media_url}, $problem) if $request->{media_url}; diff --git a/perllib/Open311/PopulateServiceList.pm b/perllib/Open311/PopulateServiceList.pm index d506111ec..9be17946e 100644 --- a/perllib/Open311/PopulateServiceList.pm +++ b/perllib/Open311/PopulateServiceList.pm @@ -1,6 +1,7 @@ package Open311::PopulateServiceList; use Moo; +use File::Basename; use Open311; has bodies => ( is => 'ro' ); @@ -128,10 +129,21 @@ sub process_service { } } +sub _action_params { + my ( $self, $action ) = @_; + + return { + editor => basename($0), + whenedited => \'current_timestamp', + note => "$action automatically by script", + }; +} + sub _handle_existing_contact { my ( $self, $contact ) = @_; my $service_name = $self->_normalize_service_name; + my $protected = $contact->get_extra_metadata("open311_protect"); print $self->_current_body->id . " already has a contact for service code " . $self->_current_service->{service_code} . "\n" if $self->verbose >= 2; @@ -139,12 +151,10 @@ sub _handle_existing_contact { eval { $contact->update( { - category => $service_name, + $protected ? () : (category => $service_name), email => $self->_current_service->{service_code}, state => 'confirmed', - editor => $0, - whenedited => \'current_timestamp', - note => 'automatically undeleted by script', + %{ $self->_action_params("undeleted") }, } ); }; @@ -160,11 +170,17 @@ sub _handle_existing_contact { if ( $contact and lc($metadata) eq 'true' ) { $self->_add_meta_to_contact( $contact ); } elsif ( $contact and $contact->extra and lc($metadata) eq 'false' ) { - $contact->set_extra_fields(); + # check if there are any protected fields that we should not delete + my @meta = ( + grep { ($_->{protected} || '') eq 'true' } + @{ $contact->get_extra_fields } + ); + $contact->set_extra_fields(@meta); $contact->update; } - $self->_set_contact_group($contact); + $self->_set_contact_group($contact) unless $protected; + $self->_set_contact_non_public($contact); push @{ $self->found_contacts }, $self->_current_service->{service_code}; } @@ -182,9 +198,7 @@ sub _create_contact { body_id => $self->_current_body->id, category => $service_name, state => 'confirmed', - editor => $0, - whenedited => \'current_timestamp', - note => 'created automatically by script', + %{ $self->_action_params("created") }, } ); }; @@ -201,6 +215,7 @@ sub _create_contact { } $self->_set_contact_group($contact); + $self->_set_contact_non_public($contact); if ( $contact ) { push @{ $self->found_contacts }, $self->_current_service->{service_code}; @@ -223,13 +238,32 @@ sub _add_meta_to_contact { return; } - # turn the data into something a bit more friendly to use + # check if there are any protected fields that we should not overwrite + my $protected = { + map { $_->{code} => $_ } + grep { ($_->{protected} || '') eq 'true' } + @{ $contact->get_extra_fields } + }; my @meta = + map { $protected->{$_->{code}} ? delete $protected->{$_->{code}} : $_ } + @{ $meta_data->{attributes} }; + + # and then add back in any protected fields that we don't fetch + push @meta, values %$protected; + + # turn the data into something a bit more friendly to use + @meta = # remove trailing colon as we add this when we display so we don't want 2 - map { $_->{description} =~ s/:\s*//; $_ } + map { + if ($_->{description}) { + $_->{description} =~ s/:\s*$//; + $_->{description} = FixMyStreet::Template::sanitize($_->{description}); + } + $_ + } # there is a display order and we only want to sort once - sort { $a->{order} <=> $b->{order} } - @{ $meta_data->{attributes} }; + sort { ($a->{order} || 0) <=> ($b->{order} || 0) } + @meta; # Some Open311 endpoints, such as Bromley and Warwickshire send <metadata> # for attributes which we *don't* want to display to the user (e.g. as @@ -260,29 +294,51 @@ sub _normalize_service_name { sub _set_contact_group { my ($self, $contact) = @_; - my $groups_enabled = $self->_current_body_cobrand && $self->_current_body_cobrand->call_hook('enable_category_groups'); - my $old_group = $contact->get_extra_metadata('group') || ''; - my $new_group = $groups_enabled ? $self->_current_service->{group} || '' : ''; - - if ($old_group ne $new_group) { - if ($new_group) { - $contact->set_extra_metadata(group => $new_group); - $contact->update({ - editor => $0, - whenedited => \'current_timestamp', - note => 'group updated automatically by script', - }); + my $old_group = $contact->groups; + my $new_group = $self->_get_new_groups; + + if ($self->_groups_different($old_group, $new_group)) { + if (@$new_group) { + $contact->set_extra_metadata(group => @$new_group == 1 ? $new_group->[0] : $new_group); + $contact->update( $self->_action_params("group updated") ); } else { $contact->unset_extra_metadata('group'); - $contact->update({ - editor => $0, - whenedited => \'current_timestamp', - note => 'group removed automatically by script', - }); + $contact->update( $self->_action_params("group removed") ); } } } +sub _set_contact_non_public { + my ($self, $contact) = @_; + + # We never want to make a private category unprivate. + return if $contact->non_public; + + my %keywords = map { $_ => 1 } split /,/, ( $self->_current_service->{keywords} || '' ); + $contact->update({ + non_public => 1, + %{ $self->_action_params("marked private") }, + }) if $keywords{private}; +} + +sub _get_new_groups { + my $self = shift; + return [] unless $self->_current_body_cobrand && $self->_current_body_cobrand->enable_category_groups; + + my $groups = $self->_current_service->{groups} || []; + return $groups if @$groups; + + my $group = $self->_current_service->{group} || []; + $group = [] if @$group == 1 && !$group->[0]; # <group></group> becomes [undef]... + return $group; +} + +sub _groups_different { + my ($self, $old, $new) = @_; + + return join( ',', sort(@$old) ) ne join( ',', sort(@$new) ); +} + sub _delete_contacts_not_in_service_list { my $self = shift; @@ -306,9 +362,7 @@ sub _delete_contacts_not_in_service_list { $found_contacts->update( { state => 'deleted', - editor => $0, - whenedited => \'current_timestamp', - note => 'automatically marked as deleted by script' + %{ $self->_action_params("marked as deleted") }, } ); } diff --git a/perllib/Open311/PostServiceRequestUpdates.pm b/perllib/Open311/PostServiceRequestUpdates.pm index 1f080b168..fadd063da 100755 --- a/perllib/Open311/PostServiceRequestUpdates.pm +++ b/perllib/Open311/PostServiceRequestUpdates.pm @@ -14,20 +14,36 @@ use Open311; use constant SEND_METHOD_OPEN311 => 'Open311'; has verbose => ( is => 'ro', default => 0 ); +has current_open311 => ( is => 'rw' ); sub send { my $self = shift; + my $bodies = $self->fetch_bodies; + foreach my $body (values %$bodies) { + $self->construct_open311($body); + $self->process_body($body); + } +} + +sub fetch_bodies { my $bodies = FixMyStreet::DB->resultset('Body')->search( { send_method => SEND_METHOD_OPEN311, send_comments => 1, - } ); - + }, { prefetch => 'body_areas' } ); + my %bodies; while ( my $body = $bodies->next ) { my $cobrand = $body->get_cobrand_handler; next if $cobrand && $cobrand->call_hook('open311_post_update_skip'); - $self->process_body($body); + $bodies{$body->id} = $body; } + return \%bodies; +} + +sub construct_open311 { + my ($self, $body) = @_; + my $o = Open311->new($self->open311_params($body)); + $self->current_open311($o); } sub open311_params { @@ -51,43 +67,59 @@ sub open311_params { sub process_body { my ($self, $body) = @_; - my $o = Open311->new( $self->open311_params($body) ); - - my $comments = FixMyStreet::DB->resultset('Comment')->to_body($body)->search( { - 'me.whensent' => undef, - 'me.external_id' => undef, - 'me.state' => 'confirmed', - 'me.confirmed' => { '!=' => undef }, - 'problem.whensent' => { '!=' => undef }, - 'problem.external_id' => { '!=' => undef }, - 'problem.send_method_used' => { -like => '%Open311%' }, - }, - { + my $params = $self->construct_query($self->verbose); + + my $db = FixMyStreet::DB->schema->storage; + $db->txn_do(sub { + my $comments = FixMyStreet::DB->resultset('Comment')->to_body($body)->search($params, { + for => \'UPDATE SKIP LOCKED', order_by => [ 'confirmed', 'id' ], - } - ); + }); - while ( my $comment = $comments->next ) { - my $cobrand = $body->get_cobrand_handler || $comment->get_cobrand_logged; - - # Some cobrands (e.g. Buckinghamshire) don't want to receive updates - # from anyone except the original problem reporter. - if ($cobrand->call_hook(should_skip_sending_update => $comment)) { - unless (defined $comment->get_extra_metadata('cobrand_skipped_sending')) { - $comment->set_extra_metadata(cobrand_skipped_sending => 1); - $comment->update; - } - next; + while ( my $comment = $comments->next ) { + $self->process_update($body, $comment); } + }); +} - next if !$self->verbose && $comment->send_fail_count && retry_timeout($comment); - - $self->process_update($body, $o, $comment, $cobrand); +sub construct_query { + my ($self, $debug) = @_; + my $params = { + 'me.whensent' => undef, + 'me.external_id' => undef, + 'me.state' => 'confirmed', + 'me.confirmed' => { '!=' => undef }, + 'me.extra' => [ undef, { -not_like => '%cobrand_skipped_sending%' } ], + 'problem.whensent' => { '!=' => undef }, + 'problem.external_id' => { '!=' => undef }, + 'problem.send_method_used' => { -like => '%Open311%' }, + }; + if (!$debug) { + $params->{'-or'} = [ + 'me.send_fail_count' => 0, + 'me.send_fail_timestamp' => { '<', \"current_timestamp - '30 minutes'::interval" }, + ]; } + return $params; } sub process_update { - my ($self, $body, $o, $comment, $cobrand) = @_; + my ($self, $body, $comment) = @_; + + my $cobrand = $body->get_cobrand_handler || $comment->get_cobrand_logged; + + # Some cobrands (e.g. Buckinghamshire) don't want to receive updates + # from anyone except the original problem reporter. + if (my $skip = $cobrand->call_hook(should_skip_sending_update => $comment)) { + if ($skip ne 'WAIT' && !defined $comment->get_extra_metadata('cobrand_skipped_sending')) { + $comment->set_extra_metadata(cobrand_skipped_sending => 1); + $comment->update; + } + $self->log($comment, 'Skipping'); + return; + } + + my $o = $self->current_open311; $cobrand->call_hook(open311_pre_send => $comment, $o); @@ -98,30 +130,21 @@ sub process_update { external_id => $id, whensent => \'current_timestamp', } ); + $self->log($comment, 'Send successful'); } else { $comment->update( { send_fail_count => $comment->send_fail_count + 1, send_fail_timestamp => \'current_timestamp', send_fail_reason => "Failed to post over Open311\n\n" . $o->error, } ); - - if ( $self->verbose && $o->error ) { - warn $o->error; - } + $self->log($comment, 'Send failed'); } } -sub retry_timeout { - my $row = shift; - - my $tz = FixMyStreet->local_time_zone; - my $now = DateTime->now( time_zone => $tz ); - my $diff = $now - $row->send_fail_timestamp; - if ( $diff->in_units( 'minutes' ) < 30 ) { - return 1; - } - - return 0; +sub log { + my ($self, $comment, $msg) = @_; + return unless $self->verbose; + STDERR->print("[fmsd] [" . $comment->id . "] $msg\n"); } 1; |