Skip to content

Commit

Permalink
Improve daemon database updating.
Browse files Browse the repository at this point in the history 
Introduce a new open311_update_missing_data cobrand hook to be used to
add/save any missing data to the report before sending via Open311, that
is wrapped in a transaction. open311_extra_data_include should now only
be used to add columns for sending, not to edit the row in any way.

When sending via Open311, any changes made pre-send (after
update_missing_data) are now automatically discarded after sending.
  • Loading branch information
@dracos
dracos committed Nov 27, 2023
1 parent 72e7c6f commit 57c8a7b
Show file tree
Hide file tree
Showing 20 changed files with 207 additions and 212 deletions.
39 changes: 19 additions & 20 deletions perllib/FixMyStreet/Cobrand/Bexley.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand All @@ -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
Expand Down Expand Up @@ -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 });

Expand Down
82 changes: 45 additions & 37 deletions perllib/FixMyStreet/Cobrand/Brent.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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;

Expand Down
26 changes: 15 additions & 11 deletions perllib/FixMyStreet/Cobrand/Bromley.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 {
Expand Down
28 changes: 18 additions & 10 deletions perllib/FixMyStreet/Cobrand/Buckinghamshire.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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) {
Expand Down
4 changes: 1 addition & 3 deletions perllib/FixMyStreet/Cobrand/Camden.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -83,8 +83,6 @@ sub open311_extra_data_include {
$row->update_extra_field({ name => 'NSGRef', description => 'NSG Ref', value => $ref });
}
}

return [];
}

sub open311_config {
Expand Down
27 changes: 15 additions & 12 deletions perllib/FixMyStreet/Cobrand/CentralBedfordshire.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down
Loading

0 comments on commit 57c8a7b

Please sign in to comment.