diff options
-rw-r--r-- | CHANGELOG.md | 2 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin.pm | 12 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin/DefectTypes.pm | 1 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Dashboard.pm | 4 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Reports.pm | 4 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/BathNES.pm | 1 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Zurich.pm | 22 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/ResultSet/Body.pm | 79 | ||||
-rw-r--r-- | perllib/FixMyStreet/SendReport/Blackhole.pm | 20 | ||||
-rw-r--r-- | t/app/controller/admin/defecttypes.t | 14 | ||||
-rw-r--r-- | t/app/sendreport/blackhole.t | 42 | ||||
-rw-r--r-- | templates/email/lincolnshire/contact.html | 38 | ||||
-rw-r--r-- | templates/email/lincolnshire/contact.txt | 11 | ||||
-rw-r--r-- | templates/web/base/admin/bodies.html | 4 | ||||
-rw-r--r-- | templates/web/base/admin/defecttypes/index.html | 3 | ||||
-rw-r--r-- | templates/web/base/dashboard/index.html | 2 | ||||
-rwxr-xr-x | templates/web/base/reports/index.html | 2 |
17 files changed, 235 insertions, 26 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index c48ef33df..a1365fda5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Clicking the "Report" header links on the homepage now focusses the #pc search input #2237 - Clearer relocation options while you’re reporting a problem #2238 + - Speed up fetching lists of bodies. #2248 - Bugfixes: - Fix display of area/pins on body page when using Bing or TonerLite map. - Do not scan through all problems to show /_dev pages. @@ -16,6 +17,7 @@ - Cobrand hook for disallowing title moderation. #2228 - Cobrand hook for per-questionnaire sending. #2231 - Add option for configuring memcache server. + - Add Blackhole send method. #2246 - Add script to list/diff template changes in core that might need applying to a cobrand. diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index 7c81251e8..1158b688a 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -2147,10 +2147,14 @@ sub check_page_allowed : Private { sub fetch_all_bodies : Private { my ($self, $c ) = @_; - my @bodies = $c->model('DB::Body')->translated->all_sorted; - if ( $c->cobrand->moniker eq 'zurich' ) { - @bodies = $c->cobrand->admin_fetch_all_bodies( @bodies ); - } + my @bodies = $c->cobrand->call_hook('admin_fetch_all_bodies') || do { + my $bodies = $c->model('DB::Body')->search(undef, { + columns => [ "id", "name", "deleted", "parent" ], + })->with_parent_name; + $bodies = $bodies->with_defect_type_count if $c->stash->{with_defect_type_count}; + $bodies->translated->all_sorted; + }; + $c->stash->{bodies} = \@bodies; return 1; diff --git a/perllib/FixMyStreet/App/Controller/Admin/DefectTypes.pm b/perllib/FixMyStreet/App/Controller/Admin/DefectTypes.pm index 5dab1da2c..ed9b40fd0 100644 --- a/perllib/FixMyStreet/App/Controller/Admin/DefectTypes.pm +++ b/perllib/FixMyStreet/App/Controller/Admin/DefectTypes.pm @@ -12,6 +12,7 @@ sub index : Path : Args(0) { my $user = $c->user; if ($user->is_superuser) { + $c->stash->{with_defect_type_count} = 1; $c->forward('/admin/fetch_all_bodies'); } elsif ( $user->from_body ) { $c->forward('load_user_body', [ $user->from_body->id ]); diff --git a/perllib/FixMyStreet/App/Controller/Dashboard.pm b/perllib/FixMyStreet/App/Controller/Dashboard.pm index 790e7ec29..aadd913ca 100644 --- a/perllib/FixMyStreet/App/Controller/Dashboard.pm +++ b/perllib/FixMyStreet/App/Controller/Dashboard.pm @@ -99,7 +99,9 @@ sub index : Path : Args(0) { $c->stash->{body_name} = join "", map { $children->{$_}->{name} } grep { $children->{$_} } $c->user->area_id; } } else { - my @bodies = $c->model('DB::Body')->active->translated->with_area_count->all_sorted; + my @bodies = $c->model('DB::Body')->search(undef, { + columns => [ "id", "name" ], + })->active->translated->with_area_count->all_sorted; $c->stash->{bodies} = \@bodies; } diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm index ade04fc7c..005397fda 100644 --- a/perllib/FixMyStreet/App/Controller/Reports.pm +++ b/perllib/FixMyStreet/App/Controller/Reports.pm @@ -66,7 +66,9 @@ sub index : Path : Args(0) { $c->stash->{children} = $children; } } else { - my @bodies = $c->model('DB::Body')->active->translated->with_area_count->all_sorted; + my @bodies = $c->model('DB::Body')->search(undef, { + columns => [ "id", "name" ], + })->active->translated->with_area_count->all_sorted; @bodies = @{$c->cobrand->call_hook('reports_hook_restrict_bodies_list', \@bodies) || \@bodies }; $c->stash->{bodies} = \@bodies; } diff --git a/perllib/FixMyStreet/Cobrand/BathNES.pm b/perllib/FixMyStreet/Cobrand/BathNES.pm index 82f79b776..c02c9328c 100644 --- a/perllib/FixMyStreet/Cobrand/BathNES.pm +++ b/perllib/FixMyStreet/Cobrand/BathNES.pm @@ -202,6 +202,7 @@ sub categories_restriction { 'me.send_method' => undef, # Open311 categories 'me.send_method' => '', # Open311 categories that have been edited in the admin 'me.send_method' => 'Email::BathNES', # Street Light Fault + 'me.send_method' => 'Blackhole', # Parks categories ] } ); } diff --git a/perllib/FixMyStreet/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm index fd0201f02..1d219ce37 100644 --- a/perllib/FixMyStreet/Cobrand/Zurich.pm +++ b/perllib/FixMyStreet/Cobrand/Zurich.pm @@ -1063,7 +1063,7 @@ sub munge_sendreport_params { } sub admin_fetch_all_bodies { - my ( $self, @bodies ) = @_; + my ( $self ) = @_; sub tree_sort { my ( $level, $id, $sorted, $out ) = @_; @@ -1073,26 +1073,30 @@ sub admin_fetch_all_bodies { if ( $level == 0 ) { @sorted = sort { # Want Zurich itself at the top. - return -1 if $sorted->{$a->id}; - return 1 if $sorted->{$b->id}; + return -1 if $sorted->{$a->{id}}; + return 1 if $sorted->{$b->{id}}; # Otherwise, by name - strcoll($a->name, $b->name) + strcoll($a->{name}, $b->{name}) } @$array; } else { - @sorted = sort { strcoll($a->name, $b->name) } @$array; + @sorted = sort { strcoll($a->{name}, $b->{name}) } @$array; } foreach ( @sorted ) { - $_->api_key( $level ); # Misuse + $_->{indent_level} = $level; push @$out, $_; - if ($sorted->{$_->id}) { - tree_sort( $level+1, $_->id, $sorted, $out ); + if ($sorted->{$_->{id}}) { + tree_sort( $level+1, $_->{id}, $sorted, $out ); } } } + my @bodies = FixMyStreet::DB->resultset('Body')->search(undef, { + columns => [ "id", "name", "deleted", "parent", "endpoint" ], + })->translated->with_children_count->all_sorted; + my %sorted; foreach (@bodies) { - my $p = $_->parent ? $_->parent->id : 0; + my $p = $_->{parent} || 0; push @{$sorted{$p}}, $_; } diff --git a/perllib/FixMyStreet/DB/ResultSet/Body.pm b/perllib/FixMyStreet/DB/ResultSet/Body.pm index 0aa3e8240..1855a45c1 100644 --- a/perllib/FixMyStreet/DB/ResultSet/Body.pm +++ b/perllib/FixMyStreet/DB/ResultSet/Body.pm @@ -41,7 +41,7 @@ This restricts the ResultSet to bodies that are not marked as deleted. sub active { my $rs = shift; - $rs->search({ deleted => 0 }); + $rs->search({ 'me.deleted' => 0 }); } =item translated @@ -61,6 +61,22 @@ sub translated { }); } +=item with_parent_name + +This adds the parent name associated with each body to the ResultSet, +in the parent_name column. + +=cut + +sub with_parent_name { + my $rs = shift; + $rs->search(undef, { + '+select' => [ 'parent.name' ], + '+as' => [ 'parent_name' ], + join => 'parent', + }); +} + =item with_area_count This adds the number of areas associated with each body to the ResultSet, @@ -78,10 +94,45 @@ sub with_area_count { }); } +=item with_defect_type_count + +This adds the number of defect types associated with each body to the +ResultSet, in the defect_type_count column. + +=cut + +sub with_defect_type_count { + my $rs = shift; + $rs->search(undef, { + '+select' => [ { count => 'defect_types.name' } ], + '+as' => [ 'defect_type_count' ], + join => 'defect_types', + distinct => 1, + }); +} + +=item with_children_count + +This adds the number of children associated with each body to the +ResultSet, in the children_count column. + +=cut + +sub with_children_count { + my $rs = shift; + $rs->search(undef, { + '+select' => [ { count => 'bodies.id' } ], + '+as' => [ 'children_count' ], + join => 'bodies', + distinct => 1, + }); +} + =item all_sorted -This returns all results, as C<all()>, but sorted by their name column -(which will be the translated names if present). +This returns all results, as C<all()>, but sorted by their name (including +the translated names, if present), and as simple hashrefs not objects, for +performance reasons. =back @@ -89,8 +140,26 @@ This returns all results, as C<all()>, but sorted by their name column sub all_sorted { my $rs = shift; - my @bodies = $rs->all; - @bodies = sort { strcoll($a->name, $b->name) } @bodies; + + # Use a HashRefInflator here to return simple hashrefs rather than full + # objects. This is quicker if you have a large number of bodies; note + # fetching only the columns you need provides even more of a speed up. + my @bodies = $rs->search(undef, { + result_class => 'DBIx::Class::ResultClass::HashRefInflator', + })->all; + @bodies = sort { strcoll($a->{msgstr} || $a->{name}, $b->{msgstr} || $b->{name}) } @bodies; + + foreach my $body (@bodies) { + $body->{parent} = { id => $body->{parent}, name => $body->{parent_name} } if $body->{parent}; + + # DEPRECATED: get_column('area_count') -> area_count + next unless defined $body->{area_count}; + $body->{get_column} = sub { + my $key = shift; + return $body->{$key}; + }; + } + return @bodies; } diff --git a/perllib/FixMyStreet/SendReport/Blackhole.pm b/perllib/FixMyStreet/SendReport/Blackhole.pm new file mode 100644 index 000000000..2c1a4fc8e --- /dev/null +++ b/perllib/FixMyStreet/SendReport/Blackhole.pm @@ -0,0 +1,20 @@ +package FixMyStreet::SendReport::Blackhole; + +use Moo; + +BEGIN { extends 'FixMyStreet::SendReport'; } + +=head2 send + +Immediately marks the report as successfully sent, but doesn't actually send +it anywhere. + +=cut + +sub send { + my $self = shift; + $self->success(1); + return 0; +} + +1; diff --git a/t/app/controller/admin/defecttypes.t b/t/app/controller/admin/defecttypes.t index e7d0e42af..1eaab8414 100644 --- a/t/app/controller/admin/defecttypes.t +++ b/t/app/controller/admin/defecttypes.t @@ -28,6 +28,20 @@ FixMyStreet::override_config { ALLOWED_COBRANDS => ['oxfordshire'], }, sub { my $body = $mech->create_body_ok( 2237, 'Oxfordshire County Council' ); + subtest 'check defect types menu available to superusers' => sub { + my $user = $mech->create_user_ok( + 'superuser@example.com', + name => 'Test Superuser', + is_superuser => 1 + ); + + $mech->log_in_ok( $user->email ); + $mech->get_ok('/admin'); + $mech->content_contains('Defect Types'); + $mech->get_ok('/admin/defecttypes'); + $mech->log_out_ok(); + }; + my $user = $mech->create_user_ok( 'oxford@example.com', name => 'Test User', diff --git a/t/app/sendreport/blackhole.t b/t/app/sendreport/blackhole.t new file mode 100644 index 000000000..4335f3675 --- /dev/null +++ b/t/app/sendreport/blackhole.t @@ -0,0 +1,42 @@ +use FixMyStreet; +use FixMyStreet::DB; +use FixMyStreet::TestMech; +use FixMyStreet::Script::Reports; + +ok( my $mech = FixMyStreet::TestMech->new, 'Created mech object' ); + +my $user = $mech->create_user_ok( 'user@example.com' ); + +my $body = $mech->create_body_ok( 2551, 'Bath and North East Somerset Council'); +$body->update({ can_be_devolved => 1 }); + +my $contact = $mech->create_contact_ok( + body_id => $body->id, + category => 'Play area safety issue', + email => 'test@example.org', + send_method => 'Blackhole', +); + +my @reports = $mech->create_problems_for_body( 1, $body->id, 'Test', { + cobrand => 'bathnes', + category => $contact->category, + user => $user, +}); +my $report = $reports[0]; + +FixMyStreet::override_config { + STAGING_FLAGS => { send_reports => 1 }, +}, sub { + subtest "Report isn't sent anywhere" => sub { + $mech->clear_emails_ok; + + FixMyStreet::Script::Reports::send(); + + ok $mech->email_count_is(0), "Report email wasn't sent"; + + $report->discard_changes; + ok $report->whensent, "Report has been marked as sent"; + }; +}; + +done_testing(); diff --git a/templates/email/lincolnshire/contact.html b/templates/email/lincolnshire/contact.html new file mode 100644 index 000000000..d9e9b060a --- /dev/null +++ b/templates/email/lincolnshire/contact.html @@ -0,0 +1,38 @@ +[% + +subject_html = subject | html; +name = form_name | html; +email_summary = "“" _ subject_html _ "” – Message from " _ name _ " on " _ host; +email_footer = "Sent via " _ host _ ", IP " _ ip; +email_columns = 1; + +PROCESS '_email_settings.html'; + +INCLUDE '_email_top.html'; + +%] + +<th style="[% td_style %][% contact_meta_style %]"> + <table [% table_reset %]> + <tr> + <th style="[% contact_th_style %]">From</th> + <td style="[% contact_td_style %]">[% name %] <<a href="mailto:[% em | html %]">[% em | html %]</a>></td> + </tr> + </table> +</th> +</tr> + +<tr> +<th style="[% td_style %][% only_column_style %]"> + <h1 style="[% h1_style %]">[% subject | html %]</h1> + [% message | html | html_para | replace('<p>', '<p style="' _ p_style _ '">') %] + [%~ IF complaint %] + <p style="[% secondary_p_style %]"> + [% complaint | html %] + - <a href="[% problem_url %]">Report</a> + - <a href="[% admin_url %]">Admin</a> + </p> + [%~ END %] +</th> + +[% INCLUDE '_email_bottom.html' %] diff --git a/templates/email/lincolnshire/contact.txt b/templates/email/lincolnshire/contact.txt new file mode 100644 index 000000000..16abb3732 --- /dev/null +++ b/templates/email/lincolnshire/contact.txt @@ -0,0 +1,11 @@ +Subject: [% site_name %] message: [% subject %] + +[% message %] + +[% IF complaint %] +[ [% complaint %] - [% problem_url %] - [% admin_url %] ] +[% END %] + +-- +Sent by contact form on [% host %]. +IP address [% ip %] diff --git a/templates/web/base/admin/bodies.html b/templates/web/base/admin/bodies.html index 9f4b81340..33ae5b25a 100644 --- a/templates/web/base/admin/bodies.html +++ b/templates/web/base/admin/bodies.html @@ -32,12 +32,12 @@ </tr> [%- FOREACH body IN bodies %] [%- SET id = body.id %] - [% NEXT IF c.cobrand.moniker == 'zurich' AND admin_type == 'dm' AND (body.parent OR body.bodies) %] + [% NEXT IF c.cobrand.moniker == 'zurich' AND admin_type == 'dm' AND (body.parent OR body.children_count) %] <tr[% IF body.deleted %] class="adminhidden"[% END %] [% IF NOT counts.$id OR counts.$id.deleted == counts.$id.c %] class="is-deleted"[% END %]> <td> [% IF c.cobrand.moniker == 'zurich' %] - [% FILTER repeat(4*body.api_key) %] [% END %] + [% FILTER repeat(4*body.indent_level) %] [% END %] [% IF admin_type == 'super' %] <a href="[% c.uri_for( 'body', id ) %]">[% body.name | html %]</a> [% ELSE %] diff --git a/templates/web/base/admin/defecttypes/index.html b/templates/web/base/admin/defecttypes/index.html index c45a09e6e..9e753edb6 100644 --- a/templates/web/base/admin/defecttypes/index.html +++ b/templates/web/base/admin/defecttypes/index.html @@ -4,8 +4,7 @@ [% FOR body IN bodies %] <li> <a href="[% c.uri_for('', body.id) %]">[% body.name %]</a> - [% defect_types_count = body.defect_types.count %] - [% IF defect_types_count %]([% defect_types_count %])[% END %] + [% IF body.defect_type_count %]([% body.defect_type_count %])[% END %] </li> [% END %] </ul> diff --git a/templates/web/base/dashboard/index.html b/templates/web/base/dashboard/index.html index cb6ca154f..5377fa938 100644 --- a/templates/web/base/dashboard/index.html +++ b/templates/web/base/dashboard/index.html @@ -50,7 +50,7 @@ <label for="ward">[% loc('Council:') %]</label> <select class="form-control" name="body" id="body"><option value=''>[% loc('All') %]</option> [% FOR b IN bodies %] - [% NEXT IF NOT b.get_column("area_count") %] + [% NEXT IF NOT b.area_count %] <option value="[% b.id %]">[% b.name %]</option> [% END %] </select> diff --git a/templates/web/base/reports/index.html b/templates/web/base/reports/index.html index 9fcf0b4c5..dfb99f089 100755 --- a/templates/web/base/reports/index.html +++ b/templates/web/base/reports/index.html @@ -79,7 +79,7 @@ <option value="">[% loc('Pick your council') %]</option> [% FOR b IN bodies # Not body as 'body' may be on stash %] <option value="[% b.id %]">[% b.name | html ~%] - [% IF NOT b.get_column("area_count") %] [% loc('(no longer exists)') %][% END ~%] + [% IF NOT b.area_count %] [% loc('(no longer exists)') %][% END ~%] </option> [% END %] </select> |