diff --git a/perllib/FixMyStreet/Cobrand/Bexley.pm b/perllib/FixMyStreet/Cobrand/Bexley.pm index 5fa88d33b8d..d390d177abf 100644 --- a/perllib/FixMyStreet/Cobrand/Bexley.pm +++ b/perllib/FixMyStreet/Cobrand/Bexley.pm @@ -118,21 +118,12 @@ sub open311_config { $params->{multi_photos} = 1; } -sub open311_extra_data_include { +sub open311_update_missing_data { my ($self, $row, $h, $contact) = @_; - my $open311_only; my $feature = $self->lookup_site_code($row); my $extra = $row->get_extra_fields; if ($contact->email =~ /^Confirm/) { - push @$open311_only, - { name => 'report_url', description => 'Report URL', - value => $h->{url} }, - { name => 'title', description => 'Title', - value => $row->title }, - { name => 'description', description => 'Detail', - value => $row->detail }; - if (!$row->get_extra_field_value('site_code')) { if (my $ref = $feature->{properties}{NSG_REF}) { $row->update_extra_field({ name => 'site_code', value => $ref, description => 'Site code' }); @@ -159,18 +150,27 @@ sub open311_extra_data_include { } if ($feature && $feature->{properties}{ADDRESS}) { - # Note subtle issue here - the first call to update_extra_field above - # updates the original extra (stored by SendReport/Open311 in - # original_extra and thus kept when it 'rolls back' after adding the - # open311-only entries), but it is changed at the end of that call, so - # a second call to update_extra_field will update extra but not the - # original_extra and thus will be lost and not available to - # open311_post_send. We'll use extra_metadata here anyway. my $address = $feature->{properties}{ADDRESS}; $address =~ s/([\w']+)/\u\L$1/g; my $town = $feature->{properties}{TOWN}; $town =~ s/([\w']+)/\u\L$1/g; - $row->set_extra_metadata(nsg => { name => $address, area => $town }); + $self->{cache_nsg} = { name => $address, area => $town }; + } +} + +sub open311_extra_data_include { + my ($self, $row, $h, $contact) = @_; + + my $open311_only; + + if ($contact->email =~ /^Confirm/) { + push @$open311_only, + { name => 'report_url', description => 'Report URL', + value => $h->{url} }, + { name => 'title', description => 'Title', + value => $row->title }, + { name => 'description', description => 'Detail', + value => $row->detail }; } # Add private comments field @@ -287,10 +287,9 @@ sub open311_post_send { $row->push_extra_fields({ name => 'uniform_id', description => 'Uniform ID', value => $row->external_id }); } - if (my $nsg = $row->get_extra_metadata('nsg')) { + if (my $nsg = $self->{cache_nsg}) { $row->push_extra_fields({ name => 'NSGName', description => 'Street name', value => $nsg->{name} }); $row->push_extra_fields({ name => 'NSGArea', description => 'Street area', value => $nsg->{area} }); - $row->unset_extra_metadata('nsg'); } $row->push_extra_fields({ name => 'fixmystreet_id', description => 'FMS reference', value => $row->id }); diff --git a/perllib/FixMyStreet/Cobrand/Brent.pm b/perllib/FixMyStreet/Cobrand/Brent.pm index 0e01e0a30fc..efcfa90ae42 100644 --- a/perllib/FixMyStreet/Cobrand/Brent.pm +++ b/perllib/FixMyStreet/Cobrand/Brent.pm @@ -373,13 +373,13 @@ sub should_skip_sending_update { } -=head2 open311_extra_data_include +=head2 open311_update_missing_data =over 4 =cut -sub open311_extra_data_include { +sub open311_update_missing_data { my ($self, $row, $h, $contact) = @_; =item * Adds NSGRef from WFS service as app doesn't include road layer for Symology @@ -390,27 +390,13 @@ WFS service at the point we're sending the report over Open311. =cut - my $open311_only; if ($contact->email =~ /^Symology/) { - if (!$row->get_extra_field_value('NSGRef')) { if (my $ref = $self->lookup_site_code($row, 'usrn')) { $row->update_extra_field({ name => 'NSGRef', description => 'NSG Ref', value => $ref }); } } -=item * Copies UnitID into the details field for the Drains and gullies category - -=cut - - if ($contact->groups->[0] eq 'Drains and gullies') { - if (my $id = $row->get_extra_field_value('UnitID')) { - $self->{brent_original_detail} = $row->detail; - my $detail = $row->detail . "\n\nukey: $id"; - $row->detail($detail); - } - } - =item * Adds NSGRef from WFS service as app doesn't include road layer for Echo Same as Symology above, but different attribute name. @@ -424,19 +410,6 @@ Same as Symology above, but different attribute name. $row->update_extra_field({ name => 'usrn', description => 'USRN', value => $ref }); } } - -=item * Adds information for constructing the description on the open311 side. - -=cut - - } elsif ($contact->email =~ /^ATAK/) { - - push @$open311_only, { name => 'title', value => $row->title }; - push @$open311_only, { name => 'report_url', value => $h->{url} }; - push @$open311_only, { name => 'detail', value => $row->detail }; - push @$open311_only, { name => 'group', value => $row->get_extra_metadata('group') || '' }; - - } =item * Adds location name from WFS service for reports in ATAK groups, if missing. @@ -454,6 +427,43 @@ Same as Symology above, but different attribute name. $row->update_extra_field({ name => 'location_name', description => 'Location name', value => $name }); } } +} + +=back + +=head2 open311_extra_data_include + +=over 4 + +=cut + +sub open311_extra_data_include { + my ($self, $row, $h, $contact) = @_; + + my $open311_only; + +=item * Copies UnitID into the details field for the Drains and gullies category + +=cut + + if ($contact->email =~ /^Symology/) { + if ($contact->groups->[0] eq 'Drains and gullies') { + if (my $id = $row->get_extra_field_value('UnitID')) { + my $detail = $row->detail . "\n\nukey: $id"; + $row->detail($detail); + } + } + +=item * Adds information for constructing the description on the open311 side. + +=cut + + } elsif ($contact->email =~ /^ATAK/) { + push @$open311_only, { name => 'title', value => $row->title }; + push @$open311_only, { name => 'report_url', value => $h->{url} }; + push @$open311_only, { name => 'detail', value => $row->detail }; + push @$open311_only, { name => 'group', value => $row->get_extra_metadata('group') || '' }; + } =item * The title field gets pushed to location fields in Echo/Symology, so include closest address @@ -481,8 +491,6 @@ We use {closest_address}->summary as this is geocoder-agnostic. =back -=cut - =head2 open311_extra_data_exclude Doesn't send UnitID for Drains and gullies category as an extra @@ -507,11 +515,10 @@ to put the UnitID in the detail field for sending sub open311_post_send { my ($self, $row) = @_; + if ($row->contact->email =~ /ATAK/ && $row->external_id) { - $row->state('investigating'); + $row->update({ state => 'investigating' }); } - - $row->detail($self->{brent_original_detail}) if $self->{brent_original_detail}; } =head2 lookup_location_name @@ -783,9 +790,10 @@ around garden_cc_check_payment_status => sub { if ($resp->{paymentResult}->{status} eq 'Success') { my $auth_details = $resp->{paymentResult}{paymentDetails}{authDetails}; - $p->set_extra_metadata( 'authCode', $auth_details->{authCode} ); - $p->set_extra_metadata( 'continuousAuditNumber', - $resp->{paymentResult}{paymentDetails}{payments}{paymentSummary}{continuousAuditNumber} ); + $p->update_extra_metadata( + authCode => $auth_details->{authCode}, + continuousAuditNumber => $resp->{paymentResult}{paymentDetails}{payments}{paymentSummary}{continuousAuditNumber}, + ); $p->update_extra_field({ name => 'payment_method', value => 'csc' }); $p->update; diff --git a/perllib/FixMyStreet/Cobrand/Bromley.pm b/perllib/FixMyStreet/Cobrand/Bromley.pm index 7a51ee81b3d..3bbbaec7236 100644 --- a/perllib/FixMyStreet/Cobrand/Bromley.pm +++ b/perllib/FixMyStreet/Cobrand/Bromley.pm @@ -256,8 +256,6 @@ sub open311_pre_send { $self->_include_user_title_in_extra($row); - $self->{bromley_original_detail} = $row->detail; - my $private_comments = $row->get_extra_metadata('private_comments'); if ($private_comments) { my $text = $row->detail . "\n\nPrivate comments: $private_comments"; @@ -306,16 +304,22 @@ sub open311_munge_update_params { sub open311_post_send { my ($self, $row, $h, $sender) = @_; - $row->detail($self->{bromley_original_detail}); my $error = $sender->error; - if ($error =~ /Cannot renew this property, a new request is required/ && $row->title eq "Garden Subscription - Renew") { - # Was created as a renewal, but due to DD delay has now expired. Switch to new subscription - $row->title("Garden Subscription - New"); - $row->update_extra_field({ name => "Subscription_Type", value => $self->waste_subscription_types->{New} }); - } - if ($error =~ /Missed Collection event already open for the property/) { - $row->state('duplicate'); - } + my $db = FixMyStreet::DB->schema->storage; + $db->txn_do(sub { + my $row2 = FixMyStreet::DB->resultset('Problem')->search({ id => $row->id }, { for => \'UPDATE' })->single; + if ($error =~ /Cannot renew this property, a new request is required/ && $row2->title eq "Garden Subscription - Renew") { + # Was created as a renewal, but due to DD delay has now expired. Switch to new subscription + $row2->title("Garden Subscription - New"); + $row2->update_extra_field({ name => "Subscription_Type", value => $self->waste_subscription_types->{New} }); + $row2->update; + $row->discard_changes; + } elsif ($error =~ /Missed Collection event already open for the property/) { + $row2->state('duplicate'); + $row2->update; + $row->discard_changes; + } + }); } sub open311_contact_meta_override { diff --git a/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm b/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm index 4c36e72a4eb..718a25e628b 100644 --- a/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm +++ b/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm @@ -147,8 +147,7 @@ sub send_questionnaires { sub open311_extra_data_exclude { [ 'road-placement' ] } - -=head2 open311_extra_data_include +=head2 open311_update_missing_data All reports sent to Alloy should have a parent asset they're associated with. This is indicated by the value in the asset_resource_id field. For certain @@ -159,11 +158,8 @@ Alloy server and use that as the parent. =cut -around open311_extra_data_include => sub { - my ($orig, $self) = (shift, shift); - my $open311_only = $self->$orig(@_); - - my ($row, $h, $contact) = @_; +sub open311_update_missing_data { + my ($self, $row, $h, $contact) = @_; # If the report doesn't already have an asset, associate it with the # closest feature from the Alloy highways network layer. @@ -174,10 +170,22 @@ around open311_extra_data_include => sub { $row->update_extra_field({ name => 'asset_resource_id', value => $item_id }); } } +} + +=head2 open311_extra_data_include + +Reports in the "Apply for Access Protection Marking" category have some +extra field values that we want to append to the report description +before it's passed to Alloy. + +=cut + +around open311_extra_data_include => sub { + my ($orig, $self) = (shift, shift); + my $open311_only = $self->$orig(@_); + + my ($row, $h, $contact) = @_; - # Reports in the "Apply for Access Protection Marking" category have some - # extra field values that we want to append to the report description - # before it's passed to Alloy if (my $address = $row->get_extra_field_value('ADDRESS_POSTCODE')) { my $phone = $row->get_extra_field_value('TELEPHONE_NUMBER') || ""; for (@$open311_only) { diff --git a/perllib/FixMyStreet/Cobrand/Camden.pm b/perllib/FixMyStreet/Cobrand/Camden.pm index 0fb0d6c87a7..c1e144f2b29 100644 --- a/perllib/FixMyStreet/Cobrand/Camden.pm +++ b/perllib/FixMyStreet/Cobrand/Camden.pm @@ -72,7 +72,7 @@ sub lookup_site_code_config { } } -sub open311_extra_data_include { +sub open311_update_missing_data { my ($self, $row, $h, $contact) = @_; # Reports made via the app probably won't have a NSGRef because we don't @@ -83,8 +83,6 @@ sub open311_extra_data_include { $row->update_extra_field({ name => 'NSGRef', description => 'NSG Ref', value => $ref }); } } - - return []; } sub open311_config { diff --git a/perllib/FixMyStreet/Cobrand/CentralBedfordshire.pm b/perllib/FixMyStreet/Cobrand/CentralBedfordshire.pm index 781551d0da9..dedf3fa7ef1 100644 --- a/perllib/FixMyStreet/Cobrand/CentralBedfordshire.pm +++ b/perllib/FixMyStreet/Cobrand/CentralBedfordshire.pm @@ -192,11 +192,25 @@ sub lookup_site_code_config { } } +sub open311_update_missing_data { + my ($self, $row, $h, $contact) = @_; + + return if $contact->email =~ /Jadu/; + + # Reports made via the app probably won't have a NSGRef because we don't + # display the road layer. Instead we'll look up the closest asset from the + # WFS service at the point we're sending the report over Open311. + if (!$row->get_extra_field_value('NSGRef')) { + if (my $ref = $self->lookup_site_code($row, 'streetref1')) { + $row->update_extra_field({ name => 'NSGRef', description => 'NSG Ref', value => $ref }); + } + } +} + sub open311_extra_data_include { my ($self, $row, $h, $contact) = @_; if (my $id = $row->get_extra_field_value('UnitID')) { - $h->{cb_original_detail} = $row->detail; $row->detail($row->detail . "\n\nUnit ID: $id"); } @@ -225,15 +239,6 @@ sub open311_extra_data_include { return $open311_only; } - # Reports made via the app probably won't have a NSGRef because we don't - # display the road layer. Instead we'll look up the closest asset from the - # WFS service at the point we're sending the report over Open311. - if (!$row->get_extra_field_value('NSGRef')) { - if (my $ref = $self->lookup_site_code($row, 'streetref1')) { - $row->update_extra_field({ name => 'NSGRef', description => 'NSG Ref', value => $ref }); - } - } - if (my $cfg = $self->feature('area_code_mapping')) {; my @areas = split ',', $row->areas; my @matches = grep { $_ } map { $cfg->{$_} } @areas; @@ -258,8 +263,6 @@ sub open311_post_send { # No post send needed for Jadu backed categories. return if $row->external_id && $row->external_id =~ /Jadu/; - $row->detail($h->{cb_original_detail}) if $h->{cb_original_detail}; - # Check Open311 was successful return unless $row->external_id; diff --git a/perllib/FixMyStreet/Cobrand/CheshireEast.pm b/perllib/FixMyStreet/Cobrand/CheshireEast.pm index 8854731a699..e02793b6a9c 100644 --- a/perllib/FixMyStreet/Cobrand/CheshireEast.pm +++ b/perllib/FixMyStreet/Cobrand/CheshireEast.pm @@ -121,19 +121,6 @@ sub enter_postcode_text { 'Enter a postcode, or a road and place name'; } -around open311_extra_data_include => sub { - my ($orig, $self, $row, $h) = @_; - my $open311_only = $self->$orig($row, $h); - - if (my $address = $row->nearest_address) { - push @$open311_only, ( - { name => 'closest_address', value => $address } - ); - } - - return $open311_only -}; - sub geocoder_munge_results { my ($self, $result) = @_; $result->{display_name} = '' unless $result->{display_name} =~ /Cheshire East/; @@ -204,14 +191,13 @@ sub council_rss_alert_options { =head2 open311_extra_data_include For reports made by staff on behalf of another user, append the staff -user's email & name to the report description. +user's email & name to the report description, and include closest_address. =cut + around open311_extra_data_include => sub { my ($orig, $self, $row, $h) = @_; - $h->{ce_original_detail} = $row->detail; - my $contributed_suffix; if (my $contributed_by = $row->get_extra_metadata("contributed_by")) { if (my $staff_user = $self->users->find({ id => $contributed_by })) { @@ -229,13 +215,13 @@ around open311_extra_data_include => sub { $row->detail($row->detail . $contributed_suffix); } + if (my $address = $row->nearest_address) { + push @$open311_only, ( + { name => 'closest_address', value => $address } + ); + } + return $open311_only; }; -sub open311_post_send { - my ($self, $row, $h) = @_; - - $row->detail($h->{ce_original_detail}); -} - 1; diff --git a/perllib/FixMyStreet/Cobrand/EastSussex.pm b/perllib/FixMyStreet/Cobrand/EastSussex.pm index 669208e06fe..21a73fe0e96 100644 --- a/perllib/FixMyStreet/Cobrand/EastSussex.pm +++ b/perllib/FixMyStreet/Cobrand/EastSussex.pm @@ -9,8 +9,6 @@ sub council_area_id { return 2224; } sub open311_extra_data { my ($self, $row, $h, $contact) = @_; - $h->{es_original_detail} = $row->detail; - my $fields = $contact->get_extra_fields; my $text = ''; for my $field ( @$fields ) { @@ -23,10 +21,4 @@ sub open311_extra_data { return (undef, ['sect_label', 'road_name', 'area_name']); } -sub open311_post_send { - my ($self, $row, $h) = @_; - - $row->detail($h->{es_original_detail}); -} - 1; diff --git a/perllib/FixMyStreet/Cobrand/Hackney.pm b/perllib/FixMyStreet/Cobrand/Hackney.pm index d81195393fd..87dae601dce 100644 --- a/perllib/FixMyStreet/Cobrand/Hackney.pm +++ b/perllib/FixMyStreet/Cobrand/Hackney.pm @@ -233,6 +233,19 @@ sub munge_sendreport_params { } } +# Bit of a hack, doing it here, but soon after this point the flag +# from get_body_sender (above) will be erased by a re-fetch. +around open311_config => sub { + my ($orig, $self, $row, $h, $open311, $contact) = @_; + $self->$orig($row, $h, $open311, $contact); + + # Make sure contact 'email' set correctly for Open311 + if (my $split_match = $row->get_extra_metadata('split_match')) { + my $code = $split_match->{$contact->email}; + $contact->email($code) if $code; + } +}; + =item * When sending via Open311, make sure closest address is included =cut @@ -271,13 +284,6 @@ sub open311_extra_data_include { } push @$open311_only, { name => 'title', value => $title }; - # Make sure contact 'email' set correctly for Open311 - if (my $split_match = $row->get_extra_metadata('split_match')) { - $row->unset_extra_metadata('split_match'); - my $code = $split_match->{$contact->email}; - $contact->email($code) if $code; - } - if (my $address = $row->nearest_address) { push @$open311_only, ( { name => 'closest_address', value => $address } diff --git a/perllib/FixMyStreet/Cobrand/Hounslow.pm b/perllib/FixMyStreet/Cobrand/Hounslow.pm index 07d899720e9..56d53ed661d 100644 --- a/perllib/FixMyStreet/Cobrand/Hounslow.pm +++ b/perllib/FixMyStreet/Cobrand/Hounslow.pm @@ -101,7 +101,7 @@ sub open311_post_send { my $sender = FixMyStreet::SendReport::Email->new( to => [ [ $e, 'Hounslow Highways' ] ] ); $sender->send($row, $h); if ($sender->success) { - $row->set_extra_metadata('hounslow_email_sent', 1); + $row->update_extra_metadata(hounslow_email_sent => 1); } } diff --git a/perllib/FixMyStreet/Cobrand/Merton.pm b/perllib/FixMyStreet/Cobrand/Merton.pm index e0d7e9f046c..9195d5db777 100644 --- a/perllib/FixMyStreet/Cobrand/Merton.pm +++ b/perllib/FixMyStreet/Cobrand/Merton.pm @@ -75,8 +75,8 @@ sub reopening_disallowed { return 1; } -sub open311_extra_data_include { - my ($self, $row, $h) = @_; +sub open311_update_missing_data { + my ($self, $row, $h, $contact) = @_; # Reports made via FMS.com or the app probably won't have a USRN # value because we don't access the USRN layer on those diff --git a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm index 47a5b9ccf35..8ca5eeef672 100644 --- a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm +++ b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm @@ -165,20 +165,12 @@ sub open311_config_updates { sub open311_pre_send { my ($self, $row, $open311) = @_; - $self->{ox_original_detail} = $row->detail; - if (my $fid = $row->get_extra_field_value('feature_id')) { my $text = "Asset Id: $fid\n\n" . $row->detail; $row->detail($text); } } -sub open311_post_send { - my ($self, $row, $h) = @_; - - $row->detail($self->{ox_original_detail}); -} - sub open311_munge_update_params { my ($self, $params, $comment, $body) = @_; diff --git a/perllib/FixMyStreet/Cobrand/Peterborough.pm b/perllib/FixMyStreet/Cobrand/Peterborough.pm index 31a6356cf25..7c40d42ead2 100644 --- a/perllib/FixMyStreet/Cobrand/Peterborough.pm +++ b/perllib/FixMyStreet/Cobrand/Peterborough.pm @@ -153,7 +153,7 @@ sub open311_extra_data_exclude { my ($self, $row, $h) = @_; # We need to store this as Open311 pre_send needs to check it and it will # have been removed due to this function. - $row->set_extra_metadata(pcc_witness => $row->get_extra_field_value('pcc-witness')); + $self->{cache_pcc_witness} = $row->get_extra_field_value('pcc-witness'); [ '^PCC-', '^emergency$', '^private_land$', '^extra_detail$' ] } @@ -264,6 +264,7 @@ sub get_body_sender { if ( $emails->{flytipping} ) { my $contact = $self->SUPER::get_body_sender($body, $problem)->{contact}; $problem->set_extra_metadata('flytipping_email' => $emails->{flytipping}); + $self->{cache_flytipping_email} = 1; # This is available in post_report_sent return { method => 'Email', contact => $contact}; } } @@ -282,14 +283,14 @@ sub munge_sendreport_params { } sub _witnessed_general_flytipping { - my $row = shift; - my $witness = $row->get_extra_metadata('pcc_witness') || ''; + my ($self, $row) = @_; + my $witness = $self->{cache_pcc_witness} || ''; return ($row->category eq 'General fly tipping' && $witness eq 'yes'); } sub open311_pre_send { my ($self, $row, $open311) = @_; - return 'SKIP' if _witnessed_general_flytipping($row); + return 'SKIP' if $self->_witnessed_general_flytipping($row); # This is a temporary addition to workaround an issue with the bulky goods # backend Peterborough are using. @@ -297,10 +298,6 @@ sub open311_pre_send { # manner, we concatenate all relevant details into the report's title field, # which is displayed as a service request note in Bartec. if ($row->category eq 'Bulky collection') { - # We don't want to persist our changes to the DB, so keep a copy - # of the original title so it can be restored by open311_post_send. - $self->{pboro_original_title} = $row->get_extra_field_value("title"); - my $title = "Crew notes: " . ( $row->get_extra_field_value("CREW NOTES") || "" ); $title .= "\n\nItems:\n"; @@ -318,15 +315,8 @@ sub open311_pre_send { sub open311_post_send { my ($self, $row, $h) = @_; - if ($row->category eq 'Bulky collection') { - # Restore problem's original title before it's stored to DB. - $row->update_extra_field({ name => "title", value => $self->{pboro_original_title} }); - } - # Check Open311 was successful - my $send_email = $row->external_id || _witnessed_general_flytipping($row); - # Unset here because check above used it - $row->unset_extra_metadata('pcc_witness'); + my $send_email = $row->external_id || $self->_witnessed_general_flytipping($row); return unless $send_email; my $emails = $self->feature('open311_email'); @@ -347,8 +337,7 @@ sub suppress_report_sent_email { sub post_report_sent { my ($self, $problem) = @_; - if ( $problem->get_extra_metadata('flytipping_email') ) { - $problem->unset_extra_metadata('flytipping_email'); + if ( $self->{cache_flytipping_email} ) { $self->_post_report_sent_close($problem, 'report/new/flytipping_text.html'); } } diff --git a/perllib/FixMyStreet/Cobrand/Westminster.pm b/perllib/FixMyStreet/Cobrand/Westminster.pm index a44ff0b0fd2..9f57709eec8 100644 --- a/perllib/FixMyStreet/Cobrand/Westminster.pm +++ b/perllib/FixMyStreet/Cobrand/Westminster.pm @@ -147,8 +147,8 @@ sub open311_config { $h->{account_id} = $id || '0'; } -sub open311_extra_data_include { - my ($self, $row, $h) = @_; +sub open311_update_missing_data { + my ($self, $row, $h, $contact) = @_; # Reports made via the app probably won't have a USRN because we don't # display the road layer. Instead we'll look up the closest asset from the @@ -168,8 +168,6 @@ sub open311_extra_data_include { $row->update_extra_field({ name => 'UPRN', value => $ref }); } } - - return undef; } sub lookup_site_code_config { diff --git a/perllib/FixMyStreet/Queue/Item/Report.pm b/perllib/FixMyStreet/Queue/Item/Report.pm index 50ea1987fde..40b20b82811 100644 --- a/perllib/FixMyStreet/Queue/Item/Report.pm +++ b/perllib/FixMyStreet/Queue/Item/Report.pm @@ -308,7 +308,7 @@ sub _post_send { if (@errors) { $self->report->update_send_failed( join( '|', @errors ) ); } else { - $self->report->send_state('sent'); + $self->report->update({ send_state => 'sent' }); } my $send_confirmation_email = $self->cobrand_handler->report_sent_confirmation_email($self->report); diff --git a/perllib/FixMyStreet/Roles/CobrandSLWP.pm b/perllib/FixMyStreet/Roles/CobrandSLWP.pm index 969c687ea5a..e9dc67af004 100644 --- a/perllib/FixMyStreet/Roles/CobrandSLWP.pm +++ b/perllib/FixMyStreet/Roles/CobrandSLWP.pm @@ -102,23 +102,30 @@ sub open311_extra_data_include { sub open311_post_send { my ($self, $row, $h, $sender) = @_; my $error = $sender->error; - if ($error =~ /Cannot renew this property, a new request is required/ && $row->title eq "Garden Subscription - Renew") { - # Was created as a renewal, but due to DD delay has now expired. Switch to new subscription - $row->title("Garden Subscription - New"); - $row->update_extra_field({ name => "Request_Type", value => $self->waste_subscription_types->{New} }); - } - if ($error =~ /Missed Collection event already open for the property/) { - $row->state('duplicate'); - } - - if ($error =~ /Duplicate Event! Original eventID: (\d+)/) { - my $id = $1; - my $cfg = $self->feature('echo'); - my $echo = Integrations::Echo->new(%$cfg); - my $event = $echo->GetEvent($id, 'Id'); - $row->external_id($event->{Guid}); - $sender->success(1); - } + my $db = FixMyStreet::DB->schema->storage; + $db->txn_do(sub { + my $row2 = FixMyStreet::DB->resultset('Problem')->search({ id => $row->id }, { for => \'UPDATE' })->single; + if ($error =~ /Cannot renew this property, a new request is required/ && $row2->title eq "Garden Subscription - Renew") { + # Was created as a renewal, but due to DD delay has now expired. Switch to new subscription + $row2->title("Garden Subscription - New"); + $row2->update_extra_field({ name => "Request_Type", value => $self->waste_subscription_types->{New} }); + $row2->update; + $row->discard_changes; + } elsif ($error =~ /Missed Collection event already open for the property/) { + $row2->state('duplicate'); + $row2->update; + $row->discard_changes; + } elsif ($error =~ /Duplicate Event! Original eventID: (\d+)/) { + my $id = $1; + my $cfg = $self->feature('echo'); + my $echo = Integrations::Echo->new(%$cfg); + my $event = $echo->GetEvent($id, 'Id'); + $row2->external_id($event->{Guid}); + $sender->success(1); + $row2->update; + $row->discard_changes; + } + }); } =item * No updates on waste reports diff --git a/perllib/FixMyStreet/Roles/ConfirmOpen311.pm b/perllib/FixMyStreet/Roles/ConfirmOpen311.pm index c663bfc127e..19a3f4a6a18 100644 --- a/perllib/FixMyStreet/Roles/ConfirmOpen311.pm +++ b/perllib/FixMyStreet/Roles/ConfirmOpen311.pm @@ -13,6 +13,20 @@ sub open311_config { $params->{multi_photos} = 1; } +sub open311_update_missing_data { + my ($self, $row, $h, $contact) = @_; + + # Reports made via FMS.com or the app probably won't have a USRN + # value because we don't display the adopted highways layer on those + # frontends. Instead we'll look up the closest asset from the WFS + # service at the point we're sending the report over Open311. + if (!$row->get_extra_field_value('site_code')) { + if (my $site_code = $self->lookup_site_code($row)) { + $row->update_extra_field({ name => 'site_code', value => $site_code }); + } + } +} + sub open311_extra_data_include { my ($self, $row, $h) = @_; @@ -25,16 +39,6 @@ sub open311_extra_data_include { value => $row->detail }, ]; - # Reports made via FMS.com or the app probably won't have a USRN - # value because we don't display the adopted highways layer on those - # frontends. Instead we'll look up the closest asset from the WFS - # service at the point we're sending the report over Open311. - if (!$row->get_extra_field_value('site_code')) { - if (my $site_code = $self->lookup_site_code($row)) { - $row->update_extra_field({ name => 'site_code', value => $site_code }); - } - } - return $open311_only; } diff --git a/perllib/FixMyStreet/Roles/Extra.pm b/perllib/FixMyStreet/Roles/Extra.pm index 25f913a08f4..d33d36c497b 100644 --- a/perllib/FixMyStreet/Roles/Extra.pm +++ b/perllib/FixMyStreet/Roles/Extra.pm @@ -86,27 +86,21 @@ sub unset_extra_metadata { =head2 update_extra_metadata -This calls set_extra_metadata, plus immediately updates the database with the -new data (so update in the DBIx::Class sense, not like update_extra_field), -using the PostgreSQL || operator, to not affect anything else in the column. -If you are using this, you probably don't want to then call update and -potentially overwrite other things in the extra column. +This immediately updates the database with the new data (so update in the +DBIx::Class sense, not like update_extra_field), using the PostgreSQL || +operator, to not affect anything else in the column. It then refetches the +data from the database so as to be up-to-date. Other changes on the object +will be saved, apart from any changes already made to extra. =cut sub update_extra_metadata { my ($self, %new) = @_; - $self->set_extra_metadata(%new); - my $db = $self->result_source->schema->storage; - my $table = $self->result_source->name; - $db->dbh_do(sub { - my ($storage, $dbh, $json, $id) = @_; - $dbh->do("UPDATE $table SET extra = extra || ? WHERE id=?", undef, $json, $id); - }, - encode_json(\%new), - $self->id - ); + $self->update({ + extra => \[ "coalesce(extra, '{}') || ?", encode_json(\%new) ], + }); + $self->discard_changes; } =head2 get_extra_metadata diff --git a/perllib/FixMyStreet/SendReport/Email.pm b/perllib/FixMyStreet/SendReport/Email.pm index 4043de117cc..37685c8baea 100644 --- a/perllib/FixMyStreet/SendReport/Email.pm +++ b/perllib/FixMyStreet/SendReport/Email.pm @@ -122,7 +122,7 @@ sub send { $params, $sender, $nomail, $cobrand, $row->lang); unless ($result) { - $row->set_extra_metadata('sent_to' => email_list($params->{To})); + $row->update_extra_metadata(sent_to => email_list($params->{To})); $self->success(1); } else { $self->error( 'Failed to send email' ); diff --git a/perllib/FixMyStreet/SendReport/Open311.pm b/perllib/FixMyStreet/SendReport/Open311.pm index 784c452a309..f4bdfcea355 100644 --- a/perllib/FixMyStreet/SendReport/Open311.pm +++ b/perllib/FixMyStreet/SendReport/Open311.pm @@ -35,11 +35,18 @@ sub send { my $cobrand = $body->get_cobrand_handler || $row->get_cobrand_logged; $cobrand->call_hook(open311_config => $row, $h, \%open311_params, $contact); + my $db = FixMyStreet::DB->schema->storage; + $db->txn_do(sub { + my $row2 = FixMyStreet::DB->resultset('Problem')->search({ id => $row->id }, { for => \'UPDATE' })->single; + $cobrand->call_hook(open311_update_missing_data => $row2, $h, $contact); + $row2->update; + $row->discard_changes; + }); + # Try and fill in some ones that we've been asked for, but not asked the user for - my $extra = $row->get_extra_fields(); my ($include, $exclude) = $cobrand->call_hook(open311_extra_data => $row, $h, $contact); + my $extra = $row->get_extra_fields(); - my $original_extra = [ @$extra ]; push @$extra, @$include if $include; if ($exclude) { $exclude = join('|', @$exclude); @@ -77,11 +84,11 @@ sub send { $resp = $open311->send_service_request( $row, $h, $contact->email ); } - # make sure we don't save extra changes from above - $row->set_extra_fields( @$original_extra ); + # make sure we don't save any changes from above + $row->discard_changes; if ( $skip || $resp ) { - $row->external_id( $resp ); + $row->update({ external_id => $resp }); $self->success( 1 ); } else { $self->error( "Failed to send over Open311\n" ) unless $self->error;