diff options
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 10 | ||||
| -rw-r--r-- | perllib/Open311.pm | 72 | ||||
| -rw-r--r-- | perllib/Open311/GetServiceRequestUpdates.pm | 3 | ||||
| -rw-r--r-- | perllib/Open311/GetUpdates.pm | 19 | ||||
| -rw-r--r-- | perllib/Open311/PopulateServiceList.pm | 12 | ||||
| -rw-r--r-- | t/open311/getservicerequestupdates.t | 4 | ||||
| -rw-r--r-- | t/open311/populate-service-list.t | 13 | ||||
| -rw-r--r-- | templates/web/base/report/new/category_extras.html | 4 |
8 files changed, 51 insertions, 86 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index 696234d32..b60f7ee78 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -641,8 +641,14 @@ sub setup_categories_and_bodies : Private { push @category_options, $contact->category; my $metas = $contact->get_extra_fields; - $category_extras{ $contact->category } = $metas - if scalar @$metas; + if (scalar @$metas) { + foreach (@$metas) { + if ($_->{values} && $_->{values}->{value}) { + $_->{values} = [ map { { name => $_->{name}[0], key => $_->{key}[0] } } @{$_->{values}->{value}} ]; + } + } + $category_extras{ $contact->category } = $metas; + } my $body_send_method = $bodies{$contact->body_id}->send_method || ''; $c->stash->{unresponsive}{$contact->category} = $contact->body_id diff --git a/perllib/Open311.pm b/perllib/Open311.pm index fb793b027..6434be1fb 100644 --- a/perllib/Open311.pm +++ b/perllib/Open311.pm @@ -74,17 +74,10 @@ sub send_service_request { my $obj = $self->_get_xml_object( $response ); if ( $obj ) { - if ( $obj->{ request }->{ service_request_id } ) { - my $request_id = $obj->{request}->{service_request_id}; - - unless ( ref $request_id ) { - return $request_id; - } - } else { - my $token = $obj->{ request }->{ token }; - if ( $token ) { - return $self->get_service_request_id_from_token( $token ); - } + if ( my $request_id = $obj->{request}->[0]->{service_request_id} ) { + return $request_id unless ref $request_id; + } elsif ( my $token = $obj->{request}->[0]->{token} ) { + return $self->get_service_request_id_from_token( $token ); } } @@ -215,8 +208,8 @@ sub get_service_request_id_from_token { my $obj = $self->_get_xml_object( $service_token_xml ); - if ( $obj && $obj->{ request }->{ service_request_id } ) { - return $obj->{ request }->{ service_request_id }; + if ( $obj && $obj->{request}->[0]->{service_request_id} ) { + return $obj->{request}->[0]->{service_request_id}; } else { return 0; } @@ -240,15 +233,7 @@ sub get_service_request_updates { my $xml = $self->_get( $self->endpoints->{service_request_updates}, $params || undef ); my $service_requests = $self->_get_xml_object( $xml ); - my $requests; - if ( ref $service_requests->{request_update } eq 'ARRAY' ) { - $requests = $service_requests->{request_update}; - } - else { - $requests = [ $service_requests->{request_update} ]; - } - - return $requests; + return $service_requests->{request_update}; } sub post_service_request_update { @@ -263,16 +248,10 @@ sub post_service_request_update { my $obj = $self->_get_xml_object( $response ); if ( $obj ) { - if ( $obj->{ request_update }->{ update_id } ) { - my $update_id = $obj->{request_update}->{update_id}; - - # if there's nothing in the update_id element we get a HASHREF back - unless ( ref $update_id ) { - return $obj->{ request_update }->{ update_id }; - } + if ( my $update_id = $obj->{request_update}->[0]->{update_id} ) { + return $update_id unless ref $update_id; } else { - my $token = $obj->{ request_update }->{ token }; - if ( $token ) { + if ( my $token = $obj->{request_update}->[0]->{token} ) { return $self->get_service_request_id_from_token( $token ); } } @@ -455,7 +434,6 @@ sub _process_error { my $msg = ''; if ( ref $obj && exists $obj->{error} ) { my $errors = $obj->{error}; - $errors = [ $errors ] if ref $errors ne 'ARRAY'; $msg .= sprintf( "%s: %s\n", $_->{code}, $_->{description} ) for @{ $errors }; } @@ -463,16 +441,28 @@ sub _process_error { } sub _get_xml_object { - my $self = shift; - my $xml= shift; - - my $simple = XML::Simple->new(); - my $obj; - - eval { - $obj = $simple ->parse_string( $xml, ForceArray => [ qr/^key$/, qr/^name$/ ] ); + my ($self, $xml) = @_; + + # Of these, services/service_requests/service_request_updates are root + # elements, so GroupTags has no effect, but this is used in ForceArray too. + my $group_tags = { + services => 'service', + attributes => 'attribute', + values => 'value', + service_requests => 'request', + errors => 'error', + service_request_updates => 'request_update', + }; + my $simple = XML::Simple->new( + ForceArray => [ values %$group_tags ], + KeyAttr => {}, + GroupTags => $group_tags, + SuppressEmpty => undef, + ); + my $obj = eval { + $simple->parse_string($xml); }; - return $obj; } + 1; diff --git a/perllib/Open311/GetServiceRequestUpdates.pm b/perllib/Open311/GetServiceRequestUpdates.pm index daa5fb64d..921b2996d 100644 --- a/perllib/Open311/GetServiceRequestUpdates.pm +++ b/perllib/Open311/GetServiceRequestUpdates.pm @@ -122,8 +122,7 @@ sub update_comments { } ); - # ref test as XML::Simple will have returned an empty hashref for empty element - if ($request->{media_url} && !ref $request->{media_url}) { + if ($request->{media_url}) { my $ua = LWP::UserAgent->new; my $res = $ua->get($request->{media_url}); if ( $res->is_success && $res->content_type eq 'image/jpeg' ) { diff --git a/perllib/Open311/GetUpdates.pm b/perllib/Open311/GetUpdates.pm index 901e78809..1b1e339e3 100644 --- a/perllib/Open311/GetUpdates.pm +++ b/perllib/Open311/GetUpdates.pm @@ -42,23 +42,12 @@ sub update_reports { my ( $self, $report_ids, $open311, $body ) = @_; my $service_requests = $open311->get_service_requests( $report_ids ); - - my $requests; - - # XML::Simple is a bit inconsistent in how it structures - # things depending on the number of children an element has :( - if ( ref $service_requests->{request} eq 'ARRAY' ) { - $requests = $service_requests->{request}; - } - else { - $requests = [ $service_requests->{request} ]; - } + my $requests = $service_requests->{request}; for my $request (@$requests) { - # if it's a ref that means it's an empty element - # however, if there's no updated date then we can't - # tell if it's newer that what we have so we should skip it - next if ref $request->{updated_datetime} || ! exists $request->{updated_datetime}; + # if there's no updated date then we can't + # tell if it's newer than what we have so we should skip it + next unless $request->{updated_datetime}; my $request_id = $request->{service_request_id}; diff --git a/perllib/Open311/PopulateServiceList.pm b/perllib/Open311/PopulateServiceList.pm index 3236f3798..dd3bd65d6 100644 --- a/perllib/Open311/PopulateServiceList.pm +++ b/perllib/Open311/PopulateServiceList.pm @@ -71,8 +71,6 @@ sub process_services { $self->found_contacts( [] ); my $services = $list->{service}; - # XML might only have one result and then squashed the 'array'-ness - $services = [ $services ] unless ref $services eq 'ARRAY'; foreach my $service ( @$services ) { $self->_current_service( $service ); $self->process_service; @@ -205,13 +203,7 @@ sub _add_meta_to_contact { print "Fetching meta data for " . $self->_current_service->{service_code} . "\n" if $self->verbose >= 2; my $meta_data = $self->_current_open311->get_service_meta_info( $self->_current_service->{service_code} ); - if ( ref $meta_data->{ attributes }->{ attribute } eq 'HASH' ) { - $meta_data->{ attributes }->{ attribute } = [ - $meta_data->{ attributes }->{ attribute } - ]; - } - - if ( ! $meta_data->{attributes}->{attribute} ) { + unless ($meta_data->{attributes}) { warn sprintf( "Empty meta data for %s at %s", $self->_current_service->{service_code}, $self->_current_body->endpoint ) @@ -225,7 +217,7 @@ sub _add_meta_to_contact { map { $_->{description} =~ s/:\s*//; $_ } # there is a display order and we only want to sort once sort { $a->{order} <=> $b->{order} } - @{ $meta_data->{attributes}->{attribute} }; + @{ $meta_data->{attributes} }; # 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 diff --git a/t/open311/getservicerequestupdates.t b/t/open311/getservicerequestupdates.t index 134d5422e..e13a34492 100644 --- a/t/open311/getservicerequestupdates.t +++ b/t/open311/getservicerequestupdates.t @@ -52,13 +52,13 @@ for my $test ( desc => 'basic parsing - empty element', updated_datetime => '<updated_datetime />', res => { update_id => 638344, service_request_id => 1, - status => 'open', description => 'This is a note', updated_datetime => {} } , + status => 'open', description => 'This is a note', updated_datetime => undef } , }, { desc => 'basic parsing - element with no content', updated_datetime => '<updated_datetime></updated_datetime>', res => { update_id => 638344, service_request_id => 1, - status => 'open', description => 'This is a note', updated_datetime => {} } , + status => 'open', description => 'This is a note', updated_datetime => undef } , }, { desc => 'basic parsing - element with content', diff --git a/t/open311/populate-service-list.t b/t/open311/populate-service-list.t index f001926d2..606bcbc44 100644 --- a/t/open311/populate-service-list.t +++ b/t/open311/populate-service-list.t @@ -419,7 +419,6 @@ for my $test ( ); my $service_list = get_xml_simple_object( $services_xml ); - $service_list = { service => [ $service_list->{ service } ] }; $processor->_current_open311( $o ); $processor->_current_body( $body ); @@ -690,17 +689,7 @@ sub get_standard_xml { sub get_xml_simple_object { my $xml = shift; - - my $simple = XML::Simple->new(); - my $obj; - - eval { - $obj = $simple->XMLin( $xml ); - }; - - die $@ if $@; - - return $obj; + return Open311->_get_xml_object($xml); } done_testing(); diff --git a/templates/web/base/report/new/category_extras.html b/templates/web/base/report/new/category_extras.html index 12ef1486f..6275cd1b7 100644 --- a/templates/web/base/report/new/category_extras.html +++ b/templates/web/base/report/new/category_extras.html @@ -17,8 +17,8 @@ [% IF meta.variable != 'false' %] [% IF meta.exists('values') %] <select name="[% meta_name %]" id="form_[% meta_name %]"[% meta.required == 'true' ? ' required' : '' %]> - [% FOR option IN meta.values.value %] - <option value="[% option.key.0 %]"[% IF option.key.0 == report_meta.$meta_name.value %] selected[% END %]>[% option.name.0 %]</option> + [% FOR option IN meta.values %] + <option value="[% option.key %]"[% IF option.key == report_meta.$meta_name.value %] selected[% END %]>[% option.name %]</option> [% END %] </select> [% ELSE %] |