diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 2 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Cobrand/Default.pm | 12 | ||||
| -rw-r--r-- | perllib/Open311.pm | 1 | ||||
| -rw-r--r-- | perllib/Open311/PopulateServiceList.pm | 16 | ||||
| -rw-r--r-- | t/open311/populate-service-list.t | 49 | ||||
| -rw-r--r-- | templates/web/base/admin/bodies/contact-form.html | 20 |
7 files changed, 17 insertions, 84 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index c838aa7bc..6a171366e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ - unset external_status_code if blank in update - Add support for account_id parameter to POST Service Request calls. - Do not overwrite/remove protected meta data. #2598 + - Spot multiple groups inside a <groups> element. * v2.6 (3rd May 2019) - New features: diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index 9a18d0e32..554fbc3b7 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -771,8 +771,6 @@ sub setup_categories_and_bodies : Private { my %category_groups = (); for my $category (@category_options) { my $group = $category->{group} // $category->get_extra_metadata('group') // ['']; - # multiple groups from open311 can contain " which upsets the html so strip them - $group =~ s/^"|"$//g; # this could be an array ref or a string my @groups = ref $group eq 'ARRAY' ? @$group : ($group); push( @{$category_groups{$_}}, $category ) for @groups; diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index 829c85f5d..d2929dcc0 100644 --- a/perllib/FixMyStreet/Cobrand/Default.pm +++ b/perllib/FixMyStreet/Cobrand/Default.pm @@ -1112,18 +1112,6 @@ sub enable_category_groups { return $self->feature('category_groups'); } -=item enable_multiple_category_groups - -Whether a category can be included in multiple groups. Required enable_category_groups -to alse be true. - -=cut - -sub enable_multiple_category_groups { - my $self = shift; - return $self->enable_category_groups && $self->feature('multiple_category_groups'); -} - sub default_problem_state { 'unconfirmed' } sub state_groups_admin { diff --git a/perllib/Open311.pm b/perllib/Open311.pm index d1659bea1..3573409e2 100644 --- a/perllib/Open311.pm +++ b/perllib/Open311.pm @@ -614,6 +614,7 @@ sub _get_xml_object { service_requests => 'request', errors => 'error', service_request_updates => 'request_update', + groups => 'group', }; my $simple = XML::Simple->new( ForceArray => [ values %$group_tags ], diff --git a/perllib/Open311/PopulateServiceList.pm b/perllib/Open311/PopulateServiceList.pm index f6e8c8668..f1bfc0f21 100644 --- a/perllib/Open311/PopulateServiceList.pm +++ b/perllib/Open311/PopulateServiceList.pm @@ -2,7 +2,6 @@ package Open311::PopulateServiceList; use Moo; use Open311; -use Text::CSV; has bodies => ( is => 'ro' ); has found_contacts => ( is => 'rw', default => sub { [] } ); @@ -282,23 +281,16 @@ sub _set_contact_group { my ($self, $contact) = @_; my $groups_enabled = $self->_current_body_cobrand && $self->_current_body_cobrand->enable_category_groups; - my $multi_groups_enabled = $self->_current_body_cobrand && $self->_current_body_cobrand->enable_multiple_category_groups; my $old_group = $contact->get_extra_metadata('group') || ''; my $new_group = $groups_enabled ? $self->_current_service->{group} || '' : ''; - - if ($multi_groups_enabled && $new_group =~ /,/) { - my $csv = Text::CSV->new; - if ( $csv->parse($new_group) ) { - $new_group = [ $csv->fields ]; - } else { - warn "error parsing groups for " . $self->_current_body_cobrand->moniker . "contact " . $contact->category . ": $new_group\n"; - $new_group = [ $new_group ]; - } + my $new_group_multi = $groups_enabled ? $self->_current_service->{groups} || [] : []; + if (@$new_group_multi) { + $new_group = $new_group_multi; } if ($self->_groups_different($old_group, $new_group)) { if ($new_group) { - $contact->set_extra_metadata(group => $new_group); + $contact->set_extra_metadata(group => @$new_group == 1 ? $new_group->[0] : $new_group); $contact->update({ editor => $0, whenedited => \'current_timestamp', diff --git a/t/open311/populate-service-list.t b/t/open311/populate-service-list.t index 3d3144d1f..75c468666 100644 --- a/t/open311/populate-service-list.t +++ b/t/open311/populate-service-list.t @@ -81,7 +81,6 @@ my $last_update = {}; for my $test ( { desc => 'set multiple groups for contact', enable_multi => 1, groups => ['sanitation', 'street'] }, { desc => 'groups not edited if unchanged', enable_multi => 1, groups => ['sanitation', 'street'], unchanged => 1 }, - { desc => 'multiple groups has to be configured', enable_multi => 0, groups => 'sanitation,street'}, ) { subtest $test->{desc} => sub { FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } )->delete(); @@ -95,7 +94,7 @@ for my $test ( <metadata>false</metadata> <type>realtime</type> <keywords>lorem, ipsum, dolor</keywords> - <group>sanitation,street</group> + <groups><group>sanitation</group><group>street</group></groups> </service> <service> <service_code>002</service_code> @@ -115,7 +114,6 @@ for my $test ( ALLOWED_COBRANDS => [ 'tester' ], COBRAND_FEATURES => { category_groups => { tester => 1 }, - multiple_category_groups => { tester => $test->{enable_multi} }, } }, sub { my $processor = Open311::PopulateServiceList->new(); @@ -141,7 +139,7 @@ for my $test ( }; } -subtest "set multiple groups with quoted csv" => sub { +subtest "set multiple groups with groups element" => sub { FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } )->delete(); my $services_xml = '<?xml version="1.0" encoding="utf-8"?> @@ -153,7 +151,7 @@ subtest "set multiple groups with quoted csv" => sub { <metadata>false</metadata> <type>realtime</type> <keywords>lorem, ipsum, dolor</keywords> - <group>"sanitation & cleaning",street</group> + <groups><group>sanitation & cleaning</group><group>street</group></groups> </service> </services> '; @@ -164,7 +162,6 @@ subtest "set multiple groups with quoted csv" => sub { ALLOWED_COBRANDS => [ 'tester' ], COBRAND_FEATURES => { category_groups => { tester => 1 }, - multiple_category_groups => { tester => 1 }, } }, sub { my $processor = Open311::PopulateServiceList->new(); @@ -178,46 +175,6 @@ subtest "set multiple groups with quoted csv" => sub { is_deeply $contact->get_extra->{group}, ['sanitation & cleaning','street'], "groups set correctly"; }; -subtest "set multiple groups with bad csv" => sub { - FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } )->delete(); - - my $services_xml = '<?xml version="1.0" encoding="utf-8"?> - <services> - <service> - <service_code>100</service_code> - <service_name>Cans left out 24x7</service_name> - <description>Garbage or recycling cans that have been left out for more than 24 hours after collection. Violators will be cited.</description> - <metadata>false</metadata> - <type>realtime</type> - <keywords>lorem, ipsum, dolor</keywords> - <group>"sanitation,street</group> - </service> - </services> - '; - - my $service_list = get_xml_simple_object($services_xml); - - FixMyStreet::override_config { - ALLOWED_COBRANDS => [ 'tester' ], - COBRAND_FEATURES => { - category_groups => { tester => 1 }, - multiple_category_groups => { tester => 1 }, - } - }, sub { - my $processor = Open311::PopulateServiceList->new(); - $processor->_current_body( $body ); - warning_like { - $processor->process_services( $service_list ); - } qr/error parsing groups for testercontact Cans left out 24x7: "sanitation,street/, - "warning printed for bad csv"; - }; - my $contact_count = FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } )->count(); - is $contact_count, 1, 'correct number of contacts'; - - my $contact = FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id, email => 100 } )->first; - is_deeply $contact->get_extra->{group}, ['"sanitation,street'], "groups set correctly"; -}; - subtest 'check non open311 contacts marked as deleted' => sub { FixMyStreet::DB->resultset('Contact')->search( { body_id => $body->id } )->delete(); diff --git a/templates/web/base/admin/bodies/contact-form.html b/templates/web/base/admin/bodies/contact-form.html index bc2f5dde3..d3b100e15 100644 --- a/templates/web/base/admin/bodies/contact-form.html +++ b/templates/web/base/admin/bodies/contact-form.html @@ -132,21 +132,17 @@ as well.") %] <p> <label> [% loc('Group') %] - [% IF body.get_cobrand_handler.enable_multiple_category_groups %] - [% IF contact.extra.group %] - [% FOR group IN contact.extra.group %] - <input class="form-control" type="text" name="group" value="[% group | html %]" size="30"> - [% END %] - [% ELSE %] - <input class="form-control" type="text" name="group" value="" size="30"> + [% IF contact.extra.group %] + [% FOR group IN contact.extra.group %] + <input class="form-control" type="text" name="group" value="[% group | html %]" size="30"> [% END %] - <input class="hidden-js js-group-item-template form-control" type="text" name="group" value="" size="30"> - <p class="hidden-nojs"> - <button type="button" class="btn btn--small js-group-item-add">[% loc('Add group') %]</button> - </p> [% ELSE %] - <input class="form-control" type="text" name="group" value="[% contact.extra.group.join(',') | html %]" size="30"> + <input class="form-control" type="text" name="group" value="" size="30"> [% END %] + <input class="hidden-js js-group-item-template form-control" type="text" name="group" value="" size="30"> + <p class="hidden-nojs"> + <button type="button" class="btn btn--small js-group-item-add">[% loc('Add group') %]</button> + </p> </label> </p> [% END %] |